Closed Bug 1224960 Opened 7 years ago Closed 3 months ago

Conflicting accesskey in pageinfo with Select All added by Bug 418517: selectall.accesskey: "A" and mediaSaveAs.accesskey: "A"

Categories

(Firefox :: Page Info Window, defect)

42 Branch
defect
Not set
minor

Tracking

()

RESOLVED FIXED
106 Branch
Tracking Status
firefox106 --- fixed

People

(Reporter: bugdickvl, Assigned: aminomancer)

References

Details

(Keywords: good-first-bug)

Attachments

(3 files)

Bug 418517 (39+) added a "Select All" button to the Page Info > Media window, but both "Select All" and "Save As" have the same access key "A" and you need to press Alt+A second time to reach "Save As"

There is a second mediaSaveAs2.accesskey "e" present that isn't working for me.
<!ENTITY  mediaSaveAs.accesskey "A">
<!ENTITY  mediaSaveAs2.accesskey "e">
<!ENTITY  selectall.accesskey   "A">
http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/pageInfo.dtd
http://mxr.mozilla.org/mozilla-release/source/browser/locales/en-US/chrome/browser/pageInfo.dtd
Blocks: 418517
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [good first bug]
Same problem with Firefox version 43.0.2.
I would like to work on this bug
Assignee: nobody → frost17121997
Status: NEW → ASSIGNED
Changed accesskey for the "Select All" button in the Media window of Page Info
Attachment #8722459 - Flags: review?(dolske)
I spent a few minutes confused here, because the Save As button is already using mediaSaveAs2.accesskey... Then I noticed there are *two* Save As buttons -- one uses mediaSaveAs2.accesskey and the other mediaSaveAs.accesskey. And then I was confused by thinking accesskeys were triggered by Opt- instead of Control- on OSX. It's definitely a Friday. >_<

Ah, but this was deliberate from bug 383717. Otherwise the shared accesskey is triggering the wrong button?

Your patch would seem to regress that fix (by making the two buttons have the same access key again)... But it seems to work fine for me on OS X. Let me kick off a Try build to make sure it's working on Windows. Not sure if this was a platform-specific bug, or if it's an issue that's been fixed more robustly in the 10 years since bug 383717 landed (like not having accesskeys trigger hidden/disabled buttons, maybe?).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=543addb11181


If it is broken on Windows, we could fix this by:

A) Change the access key for Select All instead, to avoid the issue in the bug. (Which is what your patch title says, but not what it does.) But I think it's weird to have the Save As command triggered by different accesskeys depending on context (ie, if multiple items are selected), so this isn't really fixing the right problem.

B) Figure out an alternate fix for bug 383717. There are a few possibilities to explore, but I think the cleanest would be to simply not have two Save As buttons. (We'll probably need to move where the buttons currently are.)

Tempted to do (B) no matter what, but I don't want to scope-creep this bug unless necessary. :)
Yeah i know about the other Save As button.
That one is visible only after the Select All button has been pressed.So, i thought having the same accessKey on both buttons would be fine(because, both the buttons can't be seen at the same time,so there won't be conflicts,although both of them do the same thing)
Ok, Try build from comment 8 WFM on my Windows 10 box. Alt-e triggers the expected Save As dialog on the first press, both when just 1 or multiple items are selected. (I also tried moving focus around first, to see if that matters, but it didn't).

Can you upload a new version of the patch that removes the now-unused mediaSaveAs.accesskey from browser/locales/en-US/chrome/browser/pageInfo.dtd, and adds "r=dolske" to the Hg commit message? Then it'll be ready to land.
Attachment #8722459 - Flags: review?(dolske) → feedback+
Hey,sorry,but due to various reasons, i can't work on this bug. Please transfer it to somebody else.
Assignee: frost17121997 → nobody
Status: ASSIGNED → NEW
Removed the now-unused mediaSaveAs.accesskey
Attachment #8765340 - Flags: review?(dolske)
Comment on attachment 8765340 [details] [diff] [review]
bug1224960-changedAccessKey.patch

Thanks for picking this up!
Attachment #8765340 - Flags: review?(dolske) → review+
Keywords: good-first-bug
Whiteboard: [good first bug]

Hello! May I work on this?

In the Page Info window, the "Select All" button and the "Save As..."
button directly next to it share the same access key (in English). This
is because the "Select All" button shares its strings with the "Select
All" menuitem. This patch just adds a new fluent string for the button
and changes the access keys so they don't overlap.

Assignee: nobody → shmediaproductions
Status: NEW → ASSIGNED

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: shmediaproductions → nobody
Status: ASSIGNED → NEW

Any idea what happened with this? Can this patch land?

Flags: needinfo?(florian)

Whoops, forgot about this. I'll rebase and land it

Assignee: nobody → shughes
Status: NEW → ASSIGNED
Pushed by shughes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/24a7b2836fdd
Resolve access key conflict in pageInfo.xhtml. r=fluent-reviewers,flod
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 106 Branch
Flags: needinfo?(florian)
You need to log in before you can comment on or make changes to this bug.