Closed Bug 335383 Opened 14 years ago Closed 13 years ago

menu and toolbar accesskeys clash in suiterunner JS console

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: neil)

References

Details

Attachments

(2 files, 2 obsolete files)

bug 334997 introduced a menubar to suiterunner's toolkit-based JS console, but now accesskeys of the console mode toolbar clash with menu accesskeys. The overlay could fix that easily (attachment following in a minute), but bug 335375 blocks that approach from working (only the braodcasters get their accesskeys changed, but that change doesn't propagate to the toolbar buttons).
No longer depends on: 334997
Here's the patch I talked about in comment #0 - it's quite straight forward, and DOMI shows that the broadcasters get their new accesskeys assigned like intended. The toolbar buttons still have the pre-overlay accesskeys though :(
Depends on: 334997
Attached patch Remove broadcasters (obsolete) — Splinter Review
We can resolve the problem with the broadcasters by eliminating them ;-)
Assignee: guifeatures → neil
Status: NEW → ASSIGNED
Attachment #277383 - Flags: review?(gavin.sharp)
Requires attachment 277283 [details], obviously!

Note: I used the old names for the entities, because I hadn't read attachment 219757 [details] [diff] [review] first. I also had to replace the "V" access key for "Evaluate".
Attachment #219757 - Attachment is obsolete: true
Attachment #277388 - Flags: review?(kairo)
Comment on attachment 277383 [details] [diff] [review]
Remove broadcasters

This part looks OK, but you need to also change the toolkit console.css files to not style the toolbarbutton based on the observes attribute :(
Attachment #277383 - Flags: review?(gavin.sharp) → review-
Attached patch Fix up CSS tooSplinter Review
We get to use id selectors :-)
Attachment #277383 - Attachment is obsolete: true
Attachment #277404 - Flags: review?(gavin.sharp)
Comment on attachment 277404 [details] [diff] [review]
Fix up CSS too

Hmm, my Mac build really doesn't like it when I change the sort order (hangs), but it works fine on Windows. This patch doesn't impact it, anyways. r=me.
Attachment #277404 - Flags: review?(gavin.sharp) → review+
Comment on attachment 277388 [details] [diff] [review]
Overlay new access keys

This variant works fine, thanks for looking into that. r=me
Attachment #277388 - Flags: review?(kairo) → review+
Id selectors are indeed nicer, but why do have the id selectors have an ':'?
It doesn't seem to be needed and requires ugly escaping in CSS.
(In reply to comment #8)
>Id selectors are indeed nicer, but why do have the id selectors have an ':'?
I retained the existing ids for backwards compatibility.
The use of escape's in id's make them inefficient in parsing, and from a themers point of view there is no 'backwards compatility' in this way. We have to have both selectors anyway (both observer and the id).
I was referring to backwards compatibility of extensions (including SeaMonkey).
Seems like I forgot to mark this fixed last month, sorry.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.