Closed Bug 398704 Opened 15 years ago Closed 15 years ago

DOM Inspector Pseudo Classes dialog does not have access keys

Categories

(Other Applications :: DOM Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file, 3 obsolete files)

Attached patch Add the access keys (obsolete) — Splinter Review
DOM Inspector Pseudo Classes dialog completely lacks access keys.  The attached patch adds the necessary access keys.  These access keys, like the corresponding labels, are hardcoded in the XUL file.
Attachment #283698 - Flags: superreview?(neil)
Attachment #283698 - Flags: review?(comrade693+bmo)
Attachment #283698 - Flags: superreview?(neil) → superreview+
I think the access keys, unlike the labels, should be localized.  Labels do represent pseudo-classes, but access keys map to the keyboard, which isn't always English.  So the comment above the patched lines may need changing.
Hmmm, there are two points worth noting:

1. This would cuase the localized accesskeys to be shown in parenthesis besides the English labels, which is odd, at least.

2. I guess for anything but the most basic uses of the DOM Inspector, the user should have an English keyboard available.  For example, when she wants to add a new HTML element (to type in the tag name).

I'm not sure which path to take here...
Comment on attachment 283698 [details] [diff] [review]
Add the access keys

Localizers can always choose to keep the English version, but I'd like to give them the option to change it (even with the problem your brought up).
Attachment #283698 - Flags: review?(comrade693+bmo) → review-
Attached patch Add the access keys (v2) (obsolete) — Splinter Review
(In reply to comment #3)
> (From update of attachment 283698 [details] [diff] [review])
> Localizers can always choose to keep the English version, but I'd like to give
> them the option to change it (even with the problem your brought up).

OK, here's the new patch.
Attachment #283698 - Attachment is obsolete: true
Attachment #284140 - Flags: superreview?(neil)
Attachment #284140 - Flags: review?(comrade693+bmo)
Comment on attachment 284140 [details] [diff] [review]
Add the access keys (v2)

>+  <!ENTITY cbxStateHover.accesskey "h">
>+  <!ENTITY cbxStateActive.accesskey "a">
>+  <!ENTITY cbxStateFocus.accesskey "f">
I would have thought these need a localization note. sr=me with that fixed.

>\ No newline at end of file
Oops ;-)
Attachment #284140 - Flags: superreview?(neil) → superreview+
Attached patch Add the access keys (v3) (obsolete) — Splinter Review
(In reply to comment #5)
> (From update of attachment 284140 [details] [diff] [review])
> >+  <!ENTITY cbxStateHover.accesskey "h">
> >+  <!ENTITY cbxStateActive.accesskey "a">
> >+  <!ENTITY cbxStateFocus.accesskey "f">
> I would have thought these need a localization note. sr=me with that fixed.

Done.

> >\ No newline at end of file
> Oops ;-)

The original file didn't have a newline at the end!  Now it will have.  :-)
Attachment #284140 - Attachment is obsolete: true
Attachment #284495 - Flags: superreview?(neil)
Attachment #284495 - Flags: review?(comrade693+bmo)
Attachment #284140 - Flags: review?(comrade693+bmo)
Comment on attachment 284495 [details] [diff] [review]
Add the access keys (v3)

The original file does end with a new line; your previous patch "removed" it.
Attachment #284495 - Flags: superreview?(neil) → superreview+
Blocks: accesskey
Comment on attachment 284495 [details] [diff] [review]
Add the access keys (v3)

r=sdwilsh

localization notes have a certain form to them, so you should fix that before asking for approval:
http://mxr.mozilla.org/seamonkey/search?string=LOCALIZATION+NOTE
Attachment #284495 - Flags: review?(comrade693+bmo) → review+
Target Milestone: mozilla1.9 M9 → mozilla1.9 M11
Addressed the issue in comment 8.  Asking for approval...
Attachment #284495 - Attachment is obsolete: true
Attachment #293174 - Flags: superreview+
Attachment #293174 - Flags: review+
Attachment #293174 - Flags: approval1.9?
Attachment #293174 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in extensions/inspector/resources/content/viewers/dom/pseudoClassDialog.xul;
/cvsroot/mozilla/extensions/inspector/resources/content/viewers/dom/pseudoClassDialog.xul,v  <--  pseudoClassDialog.xul
new revision: 1.6; previous revision: 1.5
done
Checking in extensions/inspector/resources/locale/en-US/viewers/dom.dtd;
/cvsroot/mozilla/extensions/inspector/resources/locale/en-US/viewers/dom.dtd,v  <--  dom.dtd
new revision: 1.16; previous revision: 1.15
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.