Closed Bug 918017 Opened 6 years ago Closed 6 years ago

Entity reused for keyboard shortcut and access key in debugger.dtd

Categories

(DevTools :: Debugger, defect, P3)

defect

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
Priority: -- → P3
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)
Attached patch 2nd version of the same patch (obsolete) — Splinter Review
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)
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 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+
Attachment #8350308 - Flags: review?(nfitzgerald)
You should probably provide a version for m-c as well.
(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?
(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).
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.
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?
(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.
Any progress on this, both in the patch review and the accessibility issues with the accesskey disabling proposed by flod?
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).
Attachment #8351033 - Flags: review?(nfitzgerald) → review+
Adding checkin-needed as I don't have commit rights myself.
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/cc593f2cf2cb
Assignee: nobody → rpmdisguise-otros
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cc593f2cf2cb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Duplicate of this bug: 822095
Duplicate of this bug: 957217
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.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.