feat: Google Drive iframe support #163

rui-reazon99 · 2025-05-21T10:03:26Z

Hi, this is the first time that I've worked with Ruby.
Below is a summary of my work in this PR. Please let me know if anything needs to be corrected, thank you :)

What

Added support for Google Drive video iframe embedding

Ref

#162

gemini-code-assist

Hello @rui-reazon99, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team, gemini-code-assist here with a summary of this pull request. This PR introduces support for embedding Google Drive videos directly within markdown content. It achieves this by adding a new filter that recognizes a specific markdown syntax, converts the provided Google Drive share URL into an embeddable preview URL, and replaces the original markdown with an <iframe> element pointing to the preview URL. Additionally, the PR updates the iframe sanitization logic to explicitly allow iframes sourced from drive.google.com for video previews, ensuring the generated embeds are not removed or overly modified.

Highlights

New Feature: Google Drive Video Embeds: Adds the ability to embed Google Drive videos in markdown using a new syntax.
New Markdown Syntax: Introduces the @[gdrive_video](url) syntax to specify Google Drive videos for embedding.
Embed Filter Implementation: Adds a new HTML::Pipeline filter (GoogleDriveVideo) to detect the syntax, transform the URL, and generate the appropriate <iframe> tag.
Iframe Sanitization Update: Modifies the FilterIframe transformer to explicitly allow iframes with src attributes pointing to drive.google.com for video previews.
Test Coverage: Includes new tests to verify the correct handling and sanitization (or lack thereof) of Google Drive video embed iframes.

Changelog

Click here to see the changelog

lib/qiita/markdown.rb
- Added require statement for the new google_drive_video embed module to integrate it into the library.
lib/qiita/markdown/embed/google_drive_video.rb
- Created a new module to define constants related to Google Drive video embeds, specifically the SCRIPT_HOST (drive.google.com).
lib/qiita/markdown/filters/google_drive_video.rb
- Added a new HTML::Pipeline filter (GoogleDriveVideo) responsible for finding the @[gdrive_video](url) pattern in text nodes.
- Implemented logic to extract the URL, convert it to a Google Drive preview embed URL (/file/d/FILE_ID/preview), and replace the markdown with a standard <iframe> tag.
- Includes a check to prevent processing within pre, code, or tt tags.
lib/qiita/markdown/transformers/filter_iframe.rb
- Added Embed::GoogleDriveVideo::SCRIPT_HOST to the list of allowed iframe sources, permitting iframes from drive.google.com for video previews.
spec/qiita/markdown/processor_spec.rb
- Added new test cases specifically for handling HTML embed code for Google Drive videos.
- Tests confirm that iframes with Google Drive preview URLs are allowed and their attributes (like width) are not forcibly changed by the FilterIframe transformer.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Markdown magic, new tag appears,
Google Drive video, calms all fears.
An iframe takes hold,
A story unfolds,
Through pixels, the message endears.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Copilot

Pull Request Overview

This PR adds support for embedding Google Drive Videos by converting share URLs to embeddable iframe code and updating tests and configuration accordingly.

Added new tests in processor_spec.rb to validate the handling of Google Drive Video embed codes
Introduced a new filter (google_drive_video.rb) to convert share URLs into iframe embed tags and updated the allowed script hosts
Added a corresponding embed module and updated the main markdown file to require the new embed

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
spec/qiita/markdown/processor_spec.rb	Adds tests for Google Drive Video iframe handling
lib/qiita/markdown/transformers/filter_iframe.rb	Registers the Google Drive Video script host
lib/qiita/markdown/filters/google_drive_video.rb	Implements conversion of Google Drive share URLs to iframe embeds
lib/qiita/markdown/embed/google_drive_video.rb	Defines the SCRIPT_HOST for Google Drive Video
lib/qiita/markdown.rb	Requires the new google_drive_video embed module

Copilot · 2025-05-21T10:03:58Z

lib/qiita/markdown/filters/google_drive_video.rb

+              if embed_url
+                %(<iframe src="#{embed_url}" width="640" height="480" frameborder="0" allowfullscreen="true"></iframe>)
+              else
+                Regexp.last_match[0]


It appears the gsub block is missing a closing 'end' after the if/else block; please add an extra 'end' to properly close the block.

Suggested change

Regexp.last_match[0]

Regexp.last_match[0]

end

gemini-code-assist

Code Review

This pull request introduces a valuable feature: support for embedding Google Drive videos using a custom markdown tag @[gdrive_video](URL). The implementation approach is generally sound, integrating with the existing HTML pipeline.

I've identified a few areas for improvement, primarily concerning the robustness of URL parsing and the need for comprehensive tests for the new markdown syntax. There's also a minor typo in the PR title ("ifrmae" -> "iframe") and the description is empty, which would be good to update for clarity.

No explicit style guide was provided, so feedback regarding style adheres to common Ruby conventions and best practices.

Summary of Findings

Missing Tests for Markdown Syntax: The new markdown syntax @[gdrive_video](...) is not covered by tests. It's crucial to add tests for the Filters::GoogleDriveVideo to verify:

Successful conversion of valid Google Drive URLs.
Correct fallback behavior for invalid or unsupported Google Drive URLs (e.g., different domain, wrong path format).
Non-conversion when the tag is within pre, code, or tt elements.
Handling of http vs https in the input URL.

URL Parsing Robustness: The convert_to_embed_url method in lib/qiita/markdown/filters/google_drive_video.rb could be more robust by using Addressable::URI.parse for initial URL validation and component extraction, and by using a more specific regex ([-\w]+) for capturing Google Drive file IDs.
Hardcoded Iframe Height: The generated iframe has a hardcoded height="480". While the width becomes responsive due to FilterIframe, this fixed height might not be ideal for all video aspect ratios. This should be confirmed if it's the desired behavior.
Missing Newline at End of File: The file lib/qiita/markdown/filters/google_drive_video.rb is missing a newline character at the end. (Severity: Low, not commented directly due to review settings)
PR Title and Description: The PR title contains a typo ('ifrmae' instead of 'iframe'). The PR description is empty and could benefit from a brief summary of the feature. (Severity: Low, not commented directly due to review settings)

Merge Readiness

The pull request introduces a useful feature for embedding Google Drive videos. However, before merging, I strongly recommend addressing the high-severity issue of missing tests for the new markdown filter. Additionally, the suggestions for improving the robustness of the convert_to_embed_url method should be considered.

Once these points, particularly the test coverage, are addressed, the PR will be in much better shape. As a reviewer, I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers after making the necessary changes.

gemini-code-assist · 2025-05-21T10:05:48Z

lib/qiita/markdown/filters/google_drive_video.rb

+        def convert_to_embed_url(share_url)
+          if share_url =~ /drive\.google\.com\/file\/d\/([^\/]+)/
+            file_id = $1
+            "https://drive.google.com/file/d/#{file_id}/preview"
+          else
+            nil
+          end
+        end


The convert_to_embed_url method could be made more robust and align better with existing patterns:

URL Parsing: Instead of relying solely on regex for the share_url, consider using Addressable::URI.parse first. This would allow for more robust handling of URL components and easier validation of the host against Qiita::Markdown::Embed::GoogleDriveVideo::SCRIPT_HOST.

File ID Regex: The current regex drive\.google\.com\/file\/d\/([^\/]+) captures the file ID with ([^\/]+), which is quite broad. Google Drive file IDs typically consist of alphanumeric characters, hyphens, and underscores. A more specific regex like ([-\w]+) would be safer and more precise.

Capture Group Access: While $1 is functional, Regexp.last_match[1] is often preferred for clarity, especially if the method were to grow or be refactored.

Could we refactor this method to incorporate these points for better robustness and clarity? For example:

# private # # def convert_to_embed_url(share_url_string) # begin # uri = Addressable::URI.parse(share_url_string) # rescue Addressable::URI::InvalidURIError # return nil # end # # return nil unless uri.host == Qiita::Markdown::Embed::GoogleDriveVideo::SCRIPT_HOST # # # Match /file/d/FILE_ID/... # # Google Drive file IDs are typically alphanumeric with hyphens and underscores. # match_data = uri.path.match(/^\/file\/d\/([-\w]+)/) # return nil unless match_data # # file_id = match_data[1] # "https://#{Qiita::Markdown::Embed::GoogleDriveVideo::SCRIPT_HOST}/file/d/#{file_id}/preview" # end

This approach also centralizes the host check using the defined constant.

def convert_to_embed_url(share_url_string) begin uri = Addressable::URI.parse(share_url_string) rescue Addressable::URI::InvalidURIError return nil end return nil unless uri.host == Qiita::Markdown::Embed::GoogleDriveVideo::SCRIPT_HOST # Google Drive file IDs are typically alphanumeric with hyphens and underscores. match_data = uri.path.match(/^\/file\/d\/([-\w]+)/) return nil unless match_data file_id = match_data[1] "https://#{Qiita::Markdown::Embed::GoogleDriveVideo::SCRIPT_HOST}/file/d/#{file_id}/preview" end

gemini-code-assist · 2025-05-21T10:05:48Z

lib/qiita/markdown/filters/google_drive_video.rb

+              video_url = Regexp.last_match[:url]
+              embed_url = convert_to_embed_url(video_url)
+              if embed_url
+                %(<iframe src="#{embed_url}" width="640" height="480" frameborder="0" allowfullscreen="true"></iframe>)


The iframe dimensions width="640" height="480" are hardcoded here. I see that FilterIframe (which runs later) will change the width to "100%". However, the height="480" will persist.

Is this fixed height of 480px the desired default for all embedded Google Drive videos? If videos have different aspect ratios, this could lead to letterboxing or incorrect display.

Just wanted to confirm if this fixed height is intentional and considered appropriate for various video aspect ratios, or if there are plans or thoughts on making the height more adaptive (though that's often more complex, sometimes requiring JavaScript or CSS aspect ratio tricks).

lib/qiita/markdown/filters/google_drive_video.rb

Umekawa · 2025-06-03T10:13:53Z

Please addrequire "qiita/markdown/filters/google_drive_video" in lib/qiita/markdown.rb.

rui-reazon99 · 2025-06-04T03:50:59Z

@Umekawa
Thank you so much for your review!!!
I have pushed some changes in 73385bd based on your review.
Please let me know if there are any additional changes needed. Thank you😊

Umekawa · 2025-06-09T06:37:14Z

@rui-reazon99

Thanks for the fix based on my comment!
Sorry, it's difficult to add @[gdrive_video](url) notation(in filters/google_drive_video.rb) in this Pull Request.
Because conflicts link notation ([url](link)) and inconsistent with our existing notation conventions.
Please remove @[gdrive_video](url) notation.

Umekawa · 2025-06-10T09:18:40Z

@rui-reazon99
Thank you for the fix :)
Finally, please update the Description to match the current implementation and squash the commits into one 🙇

rui-reazon99 · 2025-06-10T10:11:34Z

@Umekawa
Thank you so much for your fast response and confirmation :)
I just squashed all the commits into 2d08b1f, and made some changes to the description of this PR.
Please let me know if there are any further changes needed. Thank you😊

Umekawa · 2025-06-11T01:03:54Z

Sorry, Please fix lint based on rubocop 🙇
And the class name is currently GoogleDriveVideo, but since it's possible to paste content other than videos, I think changing it to GoogleDrive would be better. What do you think?

lib/qiita/markdown/embed/google_drive_video.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
module Qiita
^
spec/qiita/markdown/processor_spec.rb:1558:1: C: [Correctable] Layout/TrailingWhitespace: Trailing whitespace detected.

51 files inspected, 2 offenses detected, 2 offenses autocorrectable
Error: Process completed with exit code 1.

fix: rubocop related lint fix

rui-reazon99 · 2025-06-11T07:27:15Z

@Umekawa
Thank you for your confirmation! Based on the message you provided, I fixed the two Rubocop lint errors mentioned above. 🙇‍♂️
Also appreciate your suggestion!!! It actually sounds great to me. I've included that change in this commit(085c83f).
Please let me know if there are any additional changes needed. Thank you😊

Umekawa

LGTM!
Thank you for contributing :) 🚀

rui-reazon99 · 2025-06-12T03:42:42Z

@Umekawa
Thank you so much! I'm glad I could contribute to the Qiita community :) 🤘
May I also ask if there is an estimated release date for this implementation?🙂

Copilot AI review requested due to automatic review settings May 21, 2025 10:03

gemini-code-assist bot reviewed May 21, 2025

View reviewed changes

Copilot AI reviewed May 21, 2025

View reviewed changes

gemini-code-assist bot suggested changes May 21, 2025

View reviewed changes

acronhub requested a review from Umekawa June 3, 2025 05:51

Umekawa reviewed Jun 3, 2025

View reviewed changes

lib/qiita/markdown/filters/google_drive_video.rb Outdated Show resolved Hide resolved

Umekawa reviewed Jun 3, 2025

View reviewed changes

lib/qiita/markdown/filters/google_drive_video.rb Outdated Show resolved Hide resolved

rui-reazon99 force-pushed the feat/google_drive branch from 98369e7 to 2d08b1f Compare June 10, 2025 10:09

Umekawa self-requested a review June 11, 2025 00:57

Umekawa previously approved these changes Jun 11, 2025

View reviewed changes

Umekawa self-requested a review June 11, 2025 01:01

rui-reazon99 dismissed Umekawa’s stale review via 7731066 June 11, 2025 07:04

rui-reazon99 force-pushed the feat/google_drive branch from 7731066 to 7ce41bd Compare June 11, 2025 07:18

rui-reazon99 changed the title ~~feat: Google Drive Video ifrmae support~~ feat: Google Drive iframe support Jun 11, 2025

feat: Google Drive Video ifrmae support

085c83f

fix: rubocop related lint fix

rui-reazon99 force-pushed the feat/google_drive branch from 7ce41bd to 085c83f Compare June 11, 2025 07:22

Umekawa approved these changes Jun 11, 2025

View reviewed changes

Umekawa merged commit 568655e into increments:master Jun 11, 2025
8 checks passed

rui-reazon99 mentioned this pull request Jun 12, 2025

Adding Google Drive's video iframe support #162

Closed

feat: Google Drive iframe support #163

feat: Google Drive iframe support #163

Conversation

rui-reazon99 commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Ref

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist bot May 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Umekawa commented Jun 3, 2025

Uh oh!

rui-reazon99 commented Jun 4, 2025

Uh oh!

Umekawa commented Jun 9, 2025

Uh oh!

Umekawa commented Jun 10, 2025

Uh oh!

rui-reazon99 commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Umekawa commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rui-reazon99 commented Jun 11, 2025

Uh oh!

Umekawa left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rui-reazon99 commented Jun 12, 2025

Uh oh!

Uh oh!

rui-reazon99 commented May 21, 2025 •

edited

Loading

rui-reazon99 commented Jun 10, 2025 •

edited

Loading

Umekawa commented Jun 11, 2025 •

edited

Loading

Umekawa left a comment •

edited

Loading