Jesper Kristensen Reporter
	Description • 11 years ago

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

	RickieES aka Ricardo Palomares 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.

	RickieES aka Ricardo Palomares 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.

	RickieES aka Ricardo Palomares 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.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]
	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.

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]
	Comment 5 • 11 years ago

You should probably provide a version for m-c as well.

	RickieES aka Ricardo Palomares 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?

	Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]
	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).

	RickieES aka Ricardo Palomares 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.

	Francesco Lodolo [:flod]
	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?

	Axel Hecht [:Pike]
	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.

	RickieES aka Ricardo Palomares Assignee
	Comment 11 • 11 years ago

This is the central version.

	RickieES aka Ricardo Palomares 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?

	Francesco Lodolo [: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).

	RickieES aka Ricardo Palomares Assignee
	Comment 14 • 11 years ago

Adding checkin-needed as I don't have commit rights myself.

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

https://hg.mozilla.org/integration/fx-team/rev/cc593f2cf2cb

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 16 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/cc593f2cf2cb

	Rimas Kudelis [use needinfo to ping]
	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.