335383 - menu and toolbar accesskeys clash in suiterunner JS console

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Robert Kaiser]

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).

Comment 1

[Robert Kaiser]

Attachment: overlay broadcasters with new accesskeys

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 :(

Comment 2

[[e-mail]]

Attachment: Remove broadcasters

We can resolve the problem with the broadcasters by eliminating them ;-)

Comment 3

[[e-mail]]

Attachment: Overlay new access keys

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".

Comment 4

[:Gavin Sharp [email: [e-mail]]]

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 :(

Comment 5

[[e-mail]]

Attachment: Fix up CSS too

We get to use id selectors :-)

Comment 6

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 7

[Robert Kaiser]

Comment on attachment 277388 [details] [diff] [review]
Overlay new access keys

This variant works fine, thanks for looking into that. r=me

Comment 8

[Alfred Kayser]

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.

Comment 9

[[e-mail]]

(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.

Comment 10

[Alfred Kayser]

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).

Comment 11

[[e-mail]]

I was referring to backwards compatibility of extensions (including SeaMonkey).

Comment 12

[[e-mail]]

Seems like I forgot to mark this fixed last month, sorry.