Closed
Bug 702939
Opened 13 years ago
Closed 13 years ago
Split debugger.css between theme and content stylesheets
Categories
(DevTools :: Debugger, defect)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: past, Assigned: past)
References
Details
Attachments
(1 file, 2 obsolete files)
Theme-related stylesheets are placed in browser/themes/*stripe/browser/devtools/. See the way the style inspector stylesheets are split up for an example. We should do the same for the debugger UI, if there is content-related stuff, or move it to themes otherwise.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → past
Status: NEW → ASSIGNED
Component: Developer Tools → Developer Tools: Debugger
QA Contact: developer.tools → developer.tools.debugger
Assignee | ||
Comment 1•13 years ago
|
||
Still work in progress.
Assignee | ||
Comment 2•13 years ago
|
||
I also fixed a few styling issues on all platforms while I was here. Tested on OS X/Win7/WinXP/Linux. Joe, I don't suppose you have tried the debugger yet, but can you take a look at the patch (what I put in theme css and what I left in content) and tell me if you see any issues?
Attachment #577978 -
Attachment is obsolete: true
Attachment #578317 -
Flags: review?(dcamp)
Attachment #578317 -
Flags: feedback?(jwalker)
Comment 3•13 years ago
|
||
Comment on attachment 578317 [details] [diff] [review] Working patch Review of attachment 578317 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. Only 2 comments, and perhaps we need a second opinion on the second. ::: browser/devtools/debugger/debugger.css @@ -83,5 @@ > - background-color: #fff; > - border: 1px solid #bbb; > - box-shadow: 0 -5px 10px -3px #ccc; > - -moz-margin-end: -1px; > - cursor: default; I think 'cursor', while visual, is mostly functional. It tells the user what the UI does, and using the wrong cursor isn't just visually annoying, it's misleading - so it belongs in content. @@ -95,5 @@ > - * Debugger statusbar > - */ > - > -#dbg-statusbar { > - -moz-appearance: statusbar; So this is probably contentious. What we're saying here is 'looks like a statusbar'. A status bar isn't mearly visual. It comes with expectations about how it functions. So I'm leaning to this being functional too. Put it another way - is it reasonable to expect a theme to do -moz-appearance: treetwisty; at this point? I would say probably not.
Attachment #578317 -
Flags: feedback?(jwalker) → feedback+
Assignee | ||
Comment 4•13 years ago
|
||
(In reply to Joe Walker from comment #3) > Looks good to me. > Only 2 comments, and perhaps we need a second opinion on the second. Agreed on both counts. I was already on the fence regarding cursor, anyway. Thank you!
Attachment #578317 -
Attachment is obsolete: true
Attachment #578317 -
Flags: review?(dcamp)
Attachment #578551 -
Flags: review?(dcamp)
Comment 5•13 years ago
|
||
Comment on attachment 578551 [details] [diff] [review] Working patch v2 Review of attachment 578551 [details] [diff] [review]: ----------------------------------------------------------------- I'm going to hand this off to Joe Walker.
Attachment #578551 -
Flags: review?(dcamp) → review?(jwalker)
Comment 6•13 years ago
|
||
Someone didn't read his history. What I meant was "Joe's f+ is good enough for me".
Comment 7•13 years ago
|
||
Comment on attachment 578551 [details] [diff] [review] Working patch v2 Review of attachment 578551 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/debugger/debugger.css @@ +141,5 @@ > display: -moz-box; > -moz-box-orient: vertical; > } > > /** Evidence that we might not have our opinions on the theme/content nature of -moz-appearance quite right: We just replaced something that was 'just into content' with something equivalent that is clearly theme. I suspect the truth is that the rules are not clear and that we have to use our judgement.
Attachment #578551 -
Flags: review?(jwalker) → review+
Assignee | ||
Comment 8•13 years ago
|
||
(In reply to Joe Walker from comment #7) > Comment on attachment 578551 [details] [diff] [review] [diff] [details] [review] > Working patch v2 > > Review of attachment 578551 [details] [diff] [review] [diff] [details] [review]: > ----------------------------------------------------------------- > > ::: browser/devtools/debugger/debugger.css > @@ +141,5 @@ > > display: -moz-box; > > -moz-box-orient: vertical; > > } > > > > /** > > Evidence that we might not have our opinions on the theme/content nature of > -moz-appearance quite right: We just replaced something that was 'just into > content' with something equivalent that is clearly theme. > > I suspect the truth is that the rules are not clear and that we have to use > our judgement. Yeah, it doesn't help that treetwisties in particular don't work in windows, forcing us to move them to theme, although it's something that the debugger heavily depends on not being broken. Perhaps we should add a note in the wiki page that moving stuff to theme css is occasionally done for working around platform quirks and overriding behavior, and not just because theme add-ons might want to change it.
Assignee | ||
Comment 9•13 years ago
|
||
Comment on attachment 578551 [details] [diff] [review] Working patch v2 We think this should be included when landing the debugger in m-c, so one of you guys (or both) should review it. This does not touch toolkit code.
Attachment #578551 -
Flags: review?(rcampbell)
Attachment #578551 -
Flags: review?(mihai.sucan)
Comment 10•13 years ago
|
||
Comment on attachment 578551 [details] [diff] [review] Working patch v2 Review of attachment 578551 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #578551 -
Flags: review?(mihai.sucan) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Thank you Mihai. Rob will you review this, too, or should I land it in remote-debug?
Updated•13 years ago
|
Attachment #578551 -
Flags: review?(rcampbell) → review+
Assignee | ||
Comment 12•13 years ago
|
||
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/90200db603e6
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•