Closed
Bug 918017
Opened 11 years ago
Closed 11 years ago
Entity reused for keyboard shortcut and access key in debugger.dtd
Categories
(DevTools :: Debugger, defect, P3)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: bugzilla, Assigned: rpmdisguise-nave)
References
Details
Attachments
(1 file, 2 obsolete files)
The following entities in browser/locales/en-US/chrome/browser/devtools/debugger.dtd are reused for both accesskey and keyboard shortcut. This makes it impossible to properly localize the accesskey without changing the keyboard shortcut. Besides, most entities in the file does not follow the naming conversion of ".label", ".accesskey" and ".key". debuggerUI.searchFile.key debuggerUI.searchGlobal.key debuggerUI.searchFunction.key debuggerUI.searchToken.key debuggerUI.searchGoToLine.key debuggerUI.searchVariable.key debuggerUI.focusVariables.key debuggerUI.removeAllWatch.key
Updated•11 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•11 years ago
|
||
I'm providing two patch versions, just differing in the value indentation strategy. I wasn't able to find a stable indentation pattern in the file (some parts indented values by related entities, others by the entire group of entities). This version indents almost all entity values to column 54.
Attachment #8350308 -
Flags: review?(nfitzgerald)
Assignee | ||
Comment 2•11 years ago
|
||
I'm providing two patch versions, just differing in the value indentation strategy. I wasn't able to find a stable indentation pattern in the file (some parts indented values by related entities, others by the entire group of entities). This version indents entity values so every group of related entities have their values starting at the same column, which is one space next to the longest entity name of the group.
Attachment #8350312 -
Flags: review?(nfitzgerald)
Assignee | ||
Comment 3•11 years ago
|
||
I've received feedback about accesskeys in parenthesis in the debugger context menu, so after finding this bug I'm providing these two versions of the same patch. The patchs are built against mozilla-beta channel, as we're still in the first weeks of the cycle. CCing Pike so he can say if it better to patch only Aurora and Central.
Comment 4•11 years ago
|
||
Comment on attachment 8350312 [details] [diff] [review] 2nd version of the same patch Review of attachment 8350312 [details] [diff] [review]: ----------------------------------------------------------------- I like this indentation better because if we create a longer property down the line, we won't have to re-indent everything.
Attachment #8350312 -
Flags: review?(nfitzgerald) → review+
Updated•11 years ago
|
Attachment #8350308 -
Flags: review?(nfitzgerald)
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Nick Fitzgerald [:fitzgen] from comment #5) > You should probably provide a version for m-c as well. I see changes in both debugger.xul and debugger.dtd between Beta and Aurora. Should I provide patches for all three channels then?
Comment 7•11 years ago
|
||
(In reply to Ricardo Palomares from comment #6) > (In reply to Nick Fitzgerald [:fitzgen] from comment #5) > > You should probably provide a version for m-c as well. > > I see changes in both debugger.xul and debugger.dtd between Beta and Aurora. > Should I provide patches for all three channels then? If you want the changes to stay around in future releases, you will have to provide a patch for m-c. Whether you want to patch Beta and/or Aurora depends on if you need the changes on those releases now. If you patch Beta, you will also have to patch Aurora, or else the changes will get "lost" when Aurora moves to Beta (and so one with Aurora and m-c, too).
Assignee | ||
Comment 8•11 years ago
|
||
OK, that's what I thought, then. It is a bit late here, so I will prepare the patches for Aurora and Central tomorrow. Thanks for the prompt response.
Comment 9•11 years ago
|
||
The first step is to provide a patch for mozilla-central, then you need approval to land on mozilla-aurora and mozilla-beta. Personally I don't like the idea of breaking string freeze on channels that are supposed to be string frozen. How bad would it be to just disable accesskeys on mozilla-aurora and mozilla-beta and expose only shortcuts?
Comment 10•11 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #9) > The first step is to provide a patch for mozilla-central, then you need > approval to land on mozilla-aurora and mozilla-beta. > > Personally I don't like the idea of breaking string freeze on channels that > are supposed to be string frozen. > > How bad would it be to just disable accesskeys on mozilla-aurora and > mozilla-beta and expose only shortcuts? I think that's a decent compromise, but CCing a few a11y folks to chime in.
Assignee | ||
Comment 11•11 years ago
|
||
This is the central version.
Attachment #8351033 -
Flags: review?(nfitzgerald)
Assignee | ||
Comment 12•11 years ago
|
||
Any progress on this, both in the patch review and the accessibility issues with the accesskey disabling proposed by flod?
Comment 13•11 years ago
|
||
Mozilla offices are basically closed from Dec 21 to Jan 2, I don't expect much activity before that day (or Jan 6 if people decide to get a couple more days) ;-) For the same reason this cycle will last 8 weeks instead of 6 (release day is Feb 4).
Updated•11 years ago
|
Attachment #8351033 -
Flags: review?(nfitzgerald) → review+
Assignee | ||
Comment 14•11 years ago
|
||
Adding checkin-needed as I don't have commit rights myself.
Keywords: checkin-needed
Updated•11 years ago
|
Attachment #8350308 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #8350312 -
Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/cc593f2cf2cb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Comment 19•11 years ago
|
||
Dunno if it's already been reported, but There's still a small leftover from this problem: <!ENTITY debuggerUI.autoPrettyPrint "Auto Prettify Minified Sources"> <!ENTITY debuggerUI.autoPrettyPrint.key "P"> this should actually be called *.accesskey.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•