Skip to content
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

Bump to Selenium 4 and Chrome 107 #4540

Merged
merged 28 commits into from
Dec 5, 2022
Merged

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Dec 3, 2022

Related to #4535.

Changelog Entry

Changed

Description

Update to Selenium 4 with Chrome 107.

Design

Things changed:

  • Font
    • Bolded fonts are 0.1-1 pixel taller
      • The affecting font family is Calibri, "Helvetica Neue", Arial, "sans-serif"
    • Number text (0 or 1) render slightly different
  • General modern UI upgrade for widgets
    • Button
      • No longer have gradient background
      • When not enough vertical space, the content is now properly centered and clipped
    • Checkbox now has a blue background
    • Date/time picker has an icon for their pop up
    • Text box border are now rounded
    • Text box placeholder is in more natural position, slightly more padding, in both LTR and RTL
    • Text box default size has changed
  • Outline
    • Now follow border-radius to round itself
    • Outline will now appear on top of scroll bar (if any)
  • Dashed border is now properly spaced, especially obvious when border-radius is applied
  • When using keyboard to focus on an element:
    • Chrome 80: Will always scroll the element into view
    • Chrome 107: Will scroll the element into view, if and only if it is completely offscreen
  • Padding between inline image and texts, are now correctly spaced (appear denser)
  • RTL: Text ellipsis was wrongly clipping the text despite there are enough space to show the full text

While we are reworking on some screenshots, we found some bad tests:

  • transcript.navigation.scrollIntoView.mouse.html
    • In its 4th screenshot, it should have "Activity: 2" aligned to the top and "Activity: 1" should not be in the view
    • This is because:
      1. Scroll view is at bottom, focus is on first activity
      2. Pressing DOWN arrow key will focus on the second activity and bring it into view
      3. The mechanism to bring into view should be:
        • Scroll up from current position (currently, bottommost position)
        • Stop when second activity is completely in view
      4. Thus, first activity should not be in the view, only second activity is in view
  • transcript.activityGrouping.html/45
    • The screenshot was wrong in Chrome 80
    • As carousel will tries to re-center, when it recenter the first image, it should shift 3 pixels to the left
    • However, in Chrome 80, no initial recenter was done
  • Fixed some flaky old tests because missing await uiConnected()

Specific Changes

  • Update to Selenium 4.6.0
  • Update to Chrome 107
  • Run npm test
    • Verify all screenshots
    • Update screenshots as needed
  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@compulim compulim marked this pull request as ready for review December 3, 2022 09:17
@compulim compulim merged commit 8ba9924 into microsoft:main Dec 5, 2022
@compulim compulim deleted the bump-selenium-4 branch December 5, 2022 18:36
@compulim compulim mentioned this pull request Dec 8, 2022
74 tasks
@compulim compulim mentioned this pull request Feb 15, 2023
76 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants