1805177 - [Experiment] The PDF Feature Callout message is not dismissed after pressing the “Esc” key

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P2)

Description

[Simona Rosu [:srosu], Ecosystem QA]

Attachment: PDFcallout_EscKey.gif

[Affected versions]:

• Firefox Nightly 109.0a1 (Build ID: 20221211212505)

[Affected Platforms]:

• Windows 10 x64
• macOS 11.7.1
• Ubuntu 22.04 x64

[Prerequisites]:

• Have the latest version of Firefox Nightly 109 installed and opened.
• Have the channel pref set to “release”.
• Have the “nimbus.debug” pref set to true on the “about:config” page.

[Steps to reproduce]:

1. Force enroll in the Treatment A branch of the experiment using the following link: about:studies?optin_slug=pdfjs-feature-callout&optin_branch=treatment-a&optin_collection=nimbus-preview
2. Navigate to a PDF URL (e.g. https://www.clickdimensions.com/links/TestPDFfile.pdf).
3. Press the “Esc” key.
4. Observe what happens.

[Expected result]:

• The PDF Feature Callout message is dismissed.

[Actual result]:

• Nothing happens, the PDF Feature Callout message is not dismissed.

[Notes]:

• This issue is also reproducible on the Treatment B of the experiment.
• Based on the PDF.js annotations -Fx 109 document, the callout message should be closed when the user press the “Esc” key.
• Attached is a screen recording of the issue.

Comment 1

[Shane Hughes [:aminomancer]]

Attachment: Bug 1805177 - Make Escape key dismiss Feature Callouts. r=mviar!

Set up event handlers so that pressing Escape dismisses Feature
Callouts. If an interactive element outside of the Callout is focused,
then the Escape key will not be consumed. Also consolidate all the event
handlers into a single switch statement so we won't need to continue
adding more callback bindings (they were only necessary before
encapsulation was implemented). Also change the names of a couple
formerly pseudo-private methods that we're now referencing externally.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baf3448d460b
Make Escape key dismiss Feature Callouts. r=mviar

Comment 3

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/baf3448d460b

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit auto_nag documentation.

Comment 5

[Shane Hughes [:aminomancer]]

Hey Venetia, do you think this fix is worth uplifting for 110? Nothing is technically relying on it, just an a11y issue. Uplift effort should be pretty minimal, risk is moderate

Comment 6

[Simona Rosu [:srosu], Ecosystem QA]

I‘ve verified this issue using the latest Firefox Nightly 111.0a1 (Build ID: 20230202091542) on Windows 10 x64, macOS 11.7.1, and Ubuntu 22.04 x64.

• The PDF Feature Callout message is dismissed after pressing the “Esc” key.

Comment 7

[Vtay]

If we are still on time for an uplift, I think it would be worth trying

Comment 8

[Shane Hughes [:aminomancer]]

Pretty sure 110 RC was today. But I'll file a request anyway, just in case.

Edit: Actually, I anticipated needing to merge this patch manually, since it includes many small refactors to the Feature Callout code which other patches have recently modified. I haven't done that before and probably can't figure it out by tomorrow

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:aminomancer, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox110 to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Shane Hughes [:aminomancer]]

Comment on attachment 9314955 [details]
Bug 1805177 - Make Escape key dismiss Feature Callouts. r=mviar!

Beta/Release Uplift Approval Request

• User impact if declined: Feature callout messages in Firefox View and PDF viewers will be less keyboard accessible, requiring users to Tab to the dismiss button and press Space/Enter instead of pressing Escape as users may expect.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It only touches the Feature Callout surface and browser tests
• String changes made/needed:
• Is Android affected?: No

Comment 11

[Pascal Chevrel:pascalc]

Comment on attachment 9314955 [details]
Bug 1805177 - Make Escape key dismiss Feature Callouts. r=mviar!

I am turning it into a release uplift request as we are in RC week and already built and shipped our Release Candidate. We can evaluate it for the planned dot release, thanks.

Comment 12

[Pascal Chevrel:pascalc]

Comment on attachment 9314955 [details]
Bug 1805177 - Make Escape key dismiss Feature Callouts. r=mviar!

The patch doesn't graft cleanly to the branch, P2/S3, I think this should ride the 111 train.

Comment 13

[Shane Hughes [:aminomancer]]

(In reply to Pascal Chevrel:pascalc from comment #12)

Comment on attachment 9314955 [details]
Bug 1805177 - Make Escape key dismiss Feature Callouts. r=mviar!

The patch doesn't graft cleanly to the branch, P2/S3, I think this should ride the 111 train.

Fair enough, thanks for investigating!