Last Comment Bug 335383 - menu and toolbar accesskeys clash in suiterunner JS console
: menu and toolbar accesskeys clash in suiterunner JS console
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
-- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
Depends on: 334997 335375
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-25 07:41 PDT by Robert Kaiser
Modified: 2008-07-31 04:23 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
overlay broadcasters with new accesskeys (2.14 KB, patch)
2006-04-25 07:49 PDT, Robert Kaiser
no flags Details | Diff | Splinter Review
Remove broadcasters (4.59 KB, patch)
2007-08-20 06:22 PDT, neil@parkwaycc.co.uk
gavin.sharp: review-
Details | Diff | Splinter Review
Overlay new access keys (2.24 KB, patch)
2007-08-20 06:44 PDT, neil@parkwaycc.co.uk
kairo: review+
Details | Diff | Splinter Review
Fix up CSS too (8.97 KB, patch)
2007-08-20 09:12 PDT, neil@parkwaycc.co.uk
gavin.sharp: review+
Details | Diff | Splinter Review

Description User image Robert Kaiser 2006-04-25 07:41:37 PDT
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 User image Robert Kaiser 2006-04-25 07:49:19 PDT
Created attachment 219757 [details] [diff] [review]
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 User image neil@parkwaycc.co.uk 2007-08-20 06:22:17 PDT
Created attachment 277383 [details] [diff] [review]
Remove broadcasters

We can resolve the problem with the broadcasters by eliminating them ;-)
Comment 3 User image neil@parkwaycc.co.uk 2007-08-20 06:44:38 PDT
Created attachment 277388 [details] [diff] [review]
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 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-08-20 07:49:58 PDT
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 User image neil@parkwaycc.co.uk 2007-08-20 09:12:12 PDT
Created attachment 277404 [details] [diff] [review]
Fix up CSS too

We get to use id selectors :-)
Comment 6 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2007-08-20 10:31:27 PDT
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 User image Robert Kaiser 2007-08-20 12:28:43 PDT
Comment on attachment 277388 [details] [diff] [review]
Overlay new access keys

This variant works fine, thanks for looking into that. r=me
Comment 8 User image Alfred Kayser 2007-08-22 12:06:26 PDT
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 User image neil@parkwaycc.co.uk 2007-08-22 12:45:11 PDT
(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 User image Alfred Kayser 2007-08-23 07:40:46 PDT
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 User image neil@parkwaycc.co.uk 2007-08-23 08:32:40 PDT
I was referring to backwards compatibility of extensions (including SeaMonkey).
Comment 12 User image neil@parkwaycc.co.uk 2007-09-22 14:03:46 PDT
Seems like I forgot to mark this fixed last month, sorry.

Note You need to log in before you can comment on or make changes to this bug.