1567624 - Can't close Page Info window with Ctrl+w

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect, P3)

Description

[Asif Youssuff]

Steps to reproduce:

1. Open Page Info window
2. Do ctrl+w shortcut

What happens:

Nothing

Expected result:

Window closes.

13:34.20 INFO: No more inbound revisions, bisection finished.
13:34.20 INFO: Last good revision: 0a4aefb36aff504e179121c8de0039c0df9e8106
13:34.20 INFO: First bad revision: 03aece6cda47ff158485d986e19ba230ea4c8834
13:34.20 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0a4aefb36aff504e179121c8de0039c0df9e8106&tochange=03aece6cda47ff158485d986e19ba230ea4c8834

Comment 1

[Asif Youssuff]

Chris Frey, Your patch seems to cause the regression. Can you please look into this?

Comment 2

[Florian Quèze [:florian]]

Adding needinfo on Jared who participated in bug 1517493.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1567624 - Fix key of cmd_close command, and add data-l10n-attrs so the 'key' attribute is allowed to be set from Fluent. r?florian

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecbdc21c19e6
Fix key of cmd_close command, and add data-l10n-attrs so the 'key' attribute is allowed to be set from Fluent. r=florian,fluent-reviewers,flod

Comment 5

[Natalia Csoregi [:nataliaCs]]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Are these kind of FTL changes an issue for localizers if we wanted to uplift this?

Comment 7

[Francesco Lodolo [:flod]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)

Are these kind of FTL changes an issue for localizers if we wanted to uplift this?

I can't provide a blanket statement about it.

In general, keyboard shortcuts should not be localized, but there can be exceptions due to keyboard layouts.

Uplifting this specific one would be possible, since we fall back to English, and it's a common letter. The only annoying part is that, suddenly, your beta translation might be missing a string, but I wouldn't worry too much about it.

Comment 8

[Florian Quèze [:florian]]

Comment on attachment 9083537 [details]
Bug 1567624 - Fix key of cmd_close command, and add data-l10n-attrs so the 'key' attribute is allowed to be set from Fluent. r?florian

Beta/Release Uplift Approval Request

• User impact if declined: Pressing ctrl+w won't close Page Info.
  Given that this regression was already present in the 67 and 68 releases, it doesn't matter much if it takes another 6 weeks for the fix to reach users... but it's unfortunate for esr users. I don't care strongly about uplifting this fix, but comment 7 says it's possible.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Open Page Info, try to close it using the Ctrl+w keyboard shortcut.
  Note: I couldn't reproduce this bug on Mac because the menubar made the shortcut work anyway. I would recommend testing on Windows.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk because it's a trivial change. But it's annoying that this contains a string...
• String changes made/needed: yes.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This will be a new regression for users upgrading to esr68
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9083537 [details]
Bug 1567624 - Fix key of cmd_close command, and add data-l10n-attrs so the 'key' attribute is allowed to be set from Fluent. r?florian

Based on the blessing of both Flod and Pike, approving this for 69.0b14 and 68.1esr.

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/faf1d9e6579b

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/6da286bb33b5

Comment 12

[Cristian Fogel [:cfogel]]

Confirmed issue with 68.0.2 on Windows 10.
Fix verified with: esr68(task clusker build) 69.0b14, 70.0a1 (2019-08-15) on Windows 10, Ubuntu 18.04.