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.

Sign up for GitHub

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

Jump to bottom

Don't add attribute 'aria-haspopup' when no popup is rendered for an annotation #18889

Merged
merged 1 commit into from
Oct 12, 2024
Merged

Don't add attribute 'aria-haspopup' when no popup is rendered for an annotation #18889

merged 1 commit into from
Oct 12, 2024

Conversation

calixteman
Copy link
Contributor

and for ink annotations, create the popup after the loop in order to avoid useless elements creation.

@calixteman
Copy link
Contributor Author

/botio integrationtest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/0ebf75bff2983d4/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/5a412ba147d16e7/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/0ebf75bff2983d4/output.txt

Total script time: 9.13 mins

  • Integration Tests: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/5a412ba147d16e7/output.txt

Total script time: 19.30 mins

  • Integration Tests: FAILED

Snuffleupagus
Snuffleupagus reviewed Oct 12, 2024
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

This seems like the sort of patch that could affect rendering, so running at least browsertest as well seems appropriate.

src/display/annotation_layer.js Outdated Show resolved Hide resolved
@calixteman
Don't add attribute 'aria-haspopup' when no popup is rendered for an …
7c43cb2 
…annotation

and for ink annotations, create the popup after the loop in order to avoid useless
elements creation.
@calixteman
Copy link
Contributor Author

/botio browsertest

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

Command cmd_browsertest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/d736980e5c34a94/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

Command cmd_browsertest from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/306c941369ccf2e/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/d736980e5c34a94/output.txt

Total script time: 20.71 mins

  • Regression tests: FAILED
  different ref/snapshot: 16
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/d736980e5c34a94/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/306c941369ccf2e/output.txt

Total script time: 26.10 mins

  • Regression tests: FAILED
  different ref/snapshot: 6

Image differences available at: http://54.193.163.58:8877/306c941369ccf2e/reftest-analyzer.html#web=eq.log

Snuffleupagus
Snuffleupagus approved these changes Oct 12, 2024
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thank you.

@calixteman calixteman merged commit e28dd8e into mozilla:master Oct 12, 2024
8 checks passed
@calixteman calixteman deleted the rm_useless_aria_has_popup branch October 12, 2024 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus approved these changes

Assignees
No one assigned
Labels
annotations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calixteman @moz-tools-bot @Snuffleupagus