Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

added frame code #2236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

@rpallavisharma
Copy link
Member

@rpallavisharma rpallavisharma commented Mar 22, 2025 •
edited
Loading

User description

Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, and help reviewers by making them as simple and short as possible.

added frame code

Description

added frame code

Motivation and Context

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Tests, Documentation

Description

  • Added a comprehensive Ruby test for iframe interactions.

  • Updated Ruby code snippets in multiple documentation files.

  • Improved iframe-related documentation across various languages.

  • Modified Hugo configuration to ignore specific errors.

Changes walkthrough 📝

Relevant files
Tests
frames_spec.rb
Add Ruby test for iframe interactions

examples/ruby/spec/interactions/frames_spec.rb

  • Added a new test for iframe interactions in Ruby.
  • Demonstrated switching to iframes using WebElement, name, and index.
  • Included interactions with elements inside iframes.
  • Ensured proper cleanup by quitting the browser.
    • +40/-2
    Documentation
    frames.en.md
    Update Ruby iframe examples in English documentation

    website_and_docs/content/documentation/webdriver/interactions/frames.en.md

  • Updated Ruby code snippets to reference the new test file.
  • Removed outdated Ruby examples for iframe interactions.
  • Improved consistency in iframe documentation.
    • +22/-31
    frames.ja.md
    Update Ruby iframe examples in Japanese documentation

    website_and_docs/content/documentation/webdriver/interactions/frames.ja.md

  • Updated Ruby code snippets to reference the new test file.
  • Removed outdated Ruby examples for iframe interactions.
  • Enhanced Japanese iframe documentation consistency.
    • +28/-20
    frames.pt-br.md
    Update Ruby iframe examples in Brazilian Portuguese documentation

    website_and_docs/content/documentation/webdriver/interactions/frames.pt-br.md

  • Updated Ruby code snippets to reference the new test file.
  • Removed outdated Ruby examples for iframe interactions.
  • Improved Brazilian Portuguese iframe documentation consistency.
    • +21/-37
    frames.zh-cn.md
    Update Ruby iframe examples in Chinese documentation

    website_and_docs/content/documentation/webdriver/interactions/frames.zh-cn.md

  • Updated Ruby code snippets to reference the new test file.
  • Removed outdated Ruby examples for iframe interactions.
  • Enhanced Chinese iframe documentation consistency.
    • +20/-41
    Configuration changes
    hugo.toml
    Update Hugo configuration for error handling

    website_and_docs/hugo.toml

  • Modified Hugo configuration to ignore specific errors.
  • Adjusted settings for better build error handling.
    • +1/-1
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    Copy link

    netlify bot commented Mar 22, 2025 •
    edited
    Loading

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit b1f57d5

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Duplicate Code

    The test contains duplicate code for switching to iframe using WebElement. Line 26 uses the same iframe variable that was already defined on line 14, and performs the same operations again.

     
    driver.find_element(name: 'iframe1-name')
driver.switch_to.frame(iframe)
expect(driver.page_source.include?('We Leave From Here')).to be true
    Incorrect Element Finding

    The code finds an element by name but doesn't use it. It finds an element with name 'iframe1-name' but then uses the previously stored iframe variable instead.

     
    driver.find_element(name: 'iframe1-name')
driver.switch_to.frame(iframe)

    Copy link
    Contributor

    qodo-merge-pro bot commented Mar 22, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion Impact
    Possible issue
    ✅ (削除) Fix iframe name switching (削除ここまで)
    Suggestion Impact:The commit partially addressed the issue by storing the result of find_element in a variable (iframe) before using it in switch_to.frame, which fixes the bug where the previous iframe variable was incorrectly reused

    code diff:

    - driver.find_element(name: 'iframe1-name')
+ iframe=driver.find_element(name: 'iframe1-name')
 driver.switch_to.frame(iframe)

    The code attempts to find an element by name but doesn't use the result.
    Instead, it reuses the previously defined iframe variable. You should either
    store the result of find_element in a variable and use it, or directly use the
    name in the switch_to.frame method.

    examples/ruby/spec/interactions/frames_spec.rb [24-26] 

     # Switch to iframe using name
-driver.find_element(name: 'iframe1-name')
-driver.switch_to.frame(iframe)
+driver.switch_to.frame('iframe1-name')

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    __

    Why: The code finds an element by name but doesn't use it, then incorrectly reuses the previous iframe variable. This is a significant bug that would cause the test to not properly test switching by name.

    		High
    General
    Improve test reliability

    The test is using page_source.include? to verify content, which is brittle and
    may break if the page content changes slightly. Use a more specific element
    locator and verification method instead.

    examples/ruby/spec/interactions/frames_spec.rb [13-16] 

     # Switch to iframe using WebElement
 iframe = driver.find_element(id: 'iframe1')
 driver.switch_to.frame(iframe)
-expect(driver.page_source.include?('We Leave From Here')).to be true
+expect(driver.find_element(tag_name: 'body').text).to include('We Leave From Here')
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    __

    Why: Using a more specific element locator with text content verification is more reliable than checking the entire page source, which makes the test less brittle to minor content changes.

    		Low
    • Update

    Copy link
    Member Author

    rpallavisharma commented Mar 23, 2025 •
    edited
    Loading

    Hey @aguspe ... I fixed the gem file.. and code looking at the PR 2100 example. now everything fails :( .
    sorry but whenever you get some time please help.
    also I am unable to add you as reviewer, your githandle doesn't come. I wish it did. thank you for your help.

    Copy link

    @luke-hill luke-hill left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Few minor tidy ups if you are interested

    gem 'rubocop-rspec', '~> 3.0'
    gem 'selenium-devtools', '= 0.133.0'
    gem 'selenium-webdriver', '= 4.29.1'
    gem 'selenium-devtools', '= 0.134.0'
    Copy link

    @luke-hill luke-hill Mar 25, 2025

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Traditionally exact versions can be provided just by doing '0.134.0' (Same below)

    let(:wait) { Selenium::WebDriver::Wait.new(timeout: 2) }

    it 'interacts with elements inside iframes' do
    #navigate to web page
    Copy link

    @luke-hill luke-hill Mar 25, 2025

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    The code comments might trip up IDE's as generally they have one space infront of the #

    # Switch to iframe using WebElement
    iframe = driver.find_element(id: 'iframe1')
    driver.switch_to.frame(iframe)
    expect(driver.page_source.include?('We Leave From Here')).to be true
    Copy link

    @luke-hill luke-hill Mar 25, 2025

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggested change
    expect(driver.page_source.include?('We Leave From Here')).tobetrue
    expect(driver.page_source).toinclude('We Leave From Here')

    Reason. RSpec exposes these booleans and it's a way of getting a better error message if it occurs

    # Switch to iframe using name
    iframe=driver.find_element(name: 'iframe1-name')
    driver.switch_to.frame(iframe)
    expect(driver.page_source.include?('We Leave From Here')).to be true
    Copy link

    @luke-hill luke-hill Mar 25, 2025

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggested change
    expect(driver.page_source.include?('We Leave From Here')).tobetrue
    expect(driver.page_source).toinclude('We Leave From Here')

    Copy link
    Member Author

    Thank you very much @luke-hill
    I will go through them and make changes.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @titusfortner titusfortner Awaiting requested review from titusfortner

    @harsha509 harsha509 Awaiting requested review from harsha509

    1 more reviewer

    @luke-hill luke-hill luke-hill left review comments

    Reviewers whose approvals may not affect merge requirements

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    AltStyle によって変換されたページ (->オリジナル) /