Closed Bug 888587 Opened 7 years ago Closed 6 years ago

The security filter access key overlaps with History menu access key.

Categories

(DevTools :: Console, defect, P3)

x86_64
Windows 7
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: Optimizer, Assigned: sshagarwal)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good-first-bug][lang=xul][mentor=Optimizer])

Attachments

(1 file, 2 obsolete files)

At least on Windows, the access key used by History menu is same as the one used by Security filter in Console (S) which prevents the usage of History menu when focus is inside Console. This should be changed to something else.
Can you please check which key would be available? E is used for Edit, C for CSS. Is "u" available or some other letter?
U us the nearest free character starting from left. But we can also do Y which will be the last character (just like in the Clear button)
Priority: -- → P3
Whiteboard: [good-first-bug][lang=xul][mentor=Optimizer]
Attached patch Patch (obsolete) — Splinter Review
Attachment #788845 - Flags: review?(scrapmachines)
Comment on attachment 788845 [details] [diff] [review]
Patch

Review of attachment 788845 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Suyash,

Thank you for the patch. There are 2 things that need to be changed:

1) The idea here is to change the accesskey of the web console "Security" button and not the "History" menu's accesskeys.
2) When changing the value of the key, we have to change the name of the key too so that the localizers get to know of the change. What we usually do is to add a number at the end like changing "accesskey" to "accesskey2"
Attachment #788845 - Flags: review?(scrapmachines) → review-
(In reply to Girish Sharma [:Optimizer] from comment #4)
> 1) The idea here is to change the accesskey of the web console "Security"
> button and not the "History" menu's accesskeys.
Sorry, will do that.

> 2) When changing the value of the key, we have to change the name of the key
> too so that the localizers get to know of the change. What we usually do is
> to add a number at the end like changing "accesskey" to "accesskey2"
Sorry, but I am not sure about this, e.g. here: https://bugzilla.mozilla.org/show_bug.cgi?id=522886#c53

Am I still supposed to change the key name?
Flags: needinfo?(scrapmachines)
Attached patch Patch v2 (obsolete) — Splinter Review
As said by :flod on #l10n, in case of accesskeys (not command keys) it's not really needed, since localizers have to deal with their own set of accesskeys and conflicts.

I am leaving it as is. If this needs change, please let me know.
Attachment #788845 - Attachment is obsolete: true
Attachment #788883 - Flags: review?(scrapmachines)
Flags: needinfo?(scrapmachines)
Comment on attachment 788883 [details] [diff] [review]
Patch v2

Review of attachment 788883 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine. I will run tests to confirm that it does not break any of them.
Attachment #788883 - Flags: review?(scrapmachines) → review+
Assignee: nobody → syshagarwal
Status: NEW → ASSIGNED
try is green : https://tbpl.mozilla.org/?tree=Try&rev=17507b1a20bd

Can some real peer give it an r+ and then change the whiteboard to [land-in-fx-team] ?
Flags: needinfo?(mihai.sucan)
Please see: 

https://developer.mozilla.org/en-US/docs/XUL_Accesskey_FAQ_and_Policies

"Avoid letters with descenders, such as p, g, q, or y". Can we pick something else instead of "y"?
Flags: needinfo?(mihai.sucan)
i and u are free on windows, no idea about Linux or Mac.
(In reply to Girish Sharma [:Optimizer] from comment #10)
> i and u are free on windows, no idea about Linux or Mac.

"Avoid letters that are only one pixel wide, such as i or l". "u" is available on Linux.
Attached patch Patch v3Splinter Review
Okay, using "u".
Attachment #788883 - Attachment is obsolete: true
Attachment #790524 - Flags: review?(scrapmachines)
Attachment #790524 - Flags: review?(mihai.sucan)
Comment on attachment 790524 [details] [diff] [review]
Patch v3

Girish: please check that this is not taking over some command on Macs. On Linux it seems fine. Afaik, Ctrl-U is not used on Macs either.
Attachment #790524 - Flags: review?(mihai.sucan) → review+
will do.
Seems like Ctrl + U does not do anything and is free.
Whiteboard: [good-first-bug][lang=xul][mentor=Optimizer] → [good-first-bug][lang=xul][mentor=Optimizer][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/03e2d35cfb2c
Whiteboard: [good-first-bug][lang=xul][mentor=Optimizer][land-in-fx-team] → [good-first-bug][lang=xul][mentor=Optimizer][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/03e2d35cfb2c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [good-first-bug][lang=xul][mentor=Optimizer][fixed-in-fx-team] → [good-first-bug][lang=xul][mentor=Optimizer]
Target Milestone: --- → Firefox 26
Attachment #790524 - Flags: review?(scrapmachines)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.