1707208 - The File's path is missing from the "Allow notifications to take you to their tab" label

Status: VERIFIED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect, P2)

Description

[Rares Doghi, Desktop QA]

Attachment: 2021-04-23_14h05_35.png

[Affected platforms]:
Platforms: All

[Steps to reproduce]

1. Launch the Firefox browser and trigger the modal on a local file that has the option to take you to their tab
2. Reach the Tab where the Modal is already triggered.

[Expected result]
The Checkbox label text should be Allow notifications like this from file:// to take you to their tab

[Actual result]
The Checkbox label text is Allow notifications like this from to take you to their tab.
The path from the file requesting the permission is missing from the label text.

Comment 1

[Rares Doghi, Desktop QA]

Attachment: Cookies.html

Adding the local HTML file I used, In case youre not seeing the checkbox label, reach a different tab > Right click > Select All Tabs > Right Click > Reload all tabs.

This should trigger the modal from that tab when the user is in a different one.

Comment 2

[:Gijs (he/him)]

Hm, I was hoping bug 1706593 might fix this, but re-reading the code, I don't think it will. :-(

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1707208 - ensure that file: dialogs display something in the focus permission checkbox label and add a test, r?jaws

Now that I finally wrote a test, I also noticed that we were trying to write the
checkbox permission value when the dialog gets aborted (ie removed because the
page disappears due to another page loading or the tab/window being closed),
which then threw an exception because the event target is the window rather than
the dialog element, and dialog.querySelector in maybeSetAllowTabSwitchPermission
fails.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de55c0718e17
ensure that file: dialogs display something in the focus permission checkbox label and add a test, r=jaws

Comment 5

[Narcis Beleuzu [:NarcisB]]

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

Comment 6

[Rares Doghi, Desktop QA]

This issue is Verified as fixed in our latest Nightly build but the issue also occurs in Beta will this be uplifted to beta as well ?

Comment 7

[:Gijs (he/him)]

Comment on attachment 9219627 [details]
Bug 1707208 - ensure that file: dialogs display something in the focus permission checkbox label and add a test, r?jaws

Beta/Release Uplift Approval Request

• User impact if declined: Broken behaviour for file: URLs
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Moving 1 line of JS and fixing an exception, comes with automated tests, still early in beta
• String changes made/needed: Nope

Comment 8

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9219627 [details]
Bug 1707208 - ensure that file: dialogs display something in the focus permission checkbox label and add a test, r?jaws

Looks safe for early beta, , approved for 89 beta 9, thanks.

Comment 9

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/6b3fd269ff57

Comment 10

[Romain Testard [:RT]]

Confirming here that we (UX and PM) want this uplifted for MR1 to Beta.

Comment 11

[Rares Doghi, Desktop QA]

This issue is Verified as Fixed in 89.0b9 on Windows, Mac and Ubuntu.