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

Status: RESOLVED FIXED

(Firefox :: Page Info Window, defect)

Description

[dickvl]

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

Comment 1

[Paul]

Same problem with Firefox version 43.0.2.

Comment 2

[Paul]

Not sure if this should be posted here or somewhere else; pertains to "Select All".

"Select All" does not work.

You can select all media (Tools --> Page Info --> Media) on a page and then browse to the folder in which to save the media in, but it never saves.

Comment 3

[YF (Yang)]

(In reply to Paul from comment #2)
> Not sure if this should be posted here or somewhere else; pertains to
> "Select All".
> 
> "Select All" does not work.
> 
> You can select all media (Tools --> Page Info --> Media) on a page and then
> browse to the folder in which to save the media in, but it never saves.

This is bug 1207168.

Comment 4

[Paul]

Checked bug 1207168 and added my e-mail address to be e-mailed upon updates.

How do I delete my comment here?

Comment 5

[YF (Yang)]

(In reply to Paul from comment #4)
> Checked bug 1207168 and added my e-mail address to be e-mailed upon updates.
> 
> How do I delete my comment here?

It cannot be deleted and is not necessary, although it can be hidden by https://wiki.mozilla.org/BMO/comment_tagging. I did this, since you are worried about this.

Comment 6

[Harshal Lele]

I would like to work on this bug

Comment 7

[Harshal Lele]

Attachment: bug1224960-changedAccessKey.patch

Changed accesskey for the "Select All" button in the Media window of Page Info

Comment 8

[Justin Dolske [: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. :)

Comment 9

[Harshal Lele]

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)

Comment 10

[Justin Dolske [:Dolske]]

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.

Comment 11

[Harshal Lele]

Hey,sorry,but due to various reasons, i can't work on this bug. Please transfer it to somebody else.

Comment 12

[Deepjyoti Mondal]

Attachment: bug1224960-changedAccessKey.patch

Removed the now-unused mediaSaveAs.accesskey

Comment 13

[Justin Dolske [:Dolske]]

Comment on attachment 8765340 [details] [diff] [review]
bug1224960-changedAccessKey.patch

Thanks for picking this up!

Comment 14

[Jayati Shrivastava]

Hello! May I work on this?

Comment 16

[Shane Hughes [:aminomancer]]

Attachment: Bug 1224960 - Resolve access key conflict in pageInfo.xhtml. r=florian

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.

Comment 17

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

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Shane Hughes [:aminomancer]]

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

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/24a7b2836fdd
Resolve access key conflict in pageInfo.xhtml. r=fluent-reviewers,flod

Comment 21

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/24a7b2836fdd