Closed
Bug 1133380
Opened 10 years ago
Closed 10 years ago
about:privatebrowsing doesn't display properly in Classic any more
Categories
(SeaMonkey :: Themes, defect)
SeaMonkey
Themes
Tracking
(seamonkey2.35 fixed, seamonkey2.36 fixed)
RESOLVED
FIXED
seamonkey2.36
People
(Reporter: neil, Assigned: neil)
References
(Blocks 1 open bug)
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
philip.chee
:
review+
stefanh
:
review+
iannbugzilla
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
Bug 1125636 changed the theming of about:config which we were borrowing from.
Assignee | ||
Comment 1•10 years ago
|
||
Does this break modern? We don't have chrome://global/skin/in-content/info-pages.css there...
Flags: needinfo?(neil)
Assignee | ||
Comment 3•10 years ago
|
||
Missing CSS files get ignored, although they might generate warnings in the Error Console (but then they would for about:config anyway so that's a separate bug.)
Flags: needinfo?(neil)
You are correct, there is no warning in the Error Console for a missing CSS file when looking at about:config on current trunk. However, that bug has changed enough to break theming of the warning box for about:config, I've filed bug 1133582 on that issue.
Assignee | ||
Comment 5•10 years ago
|
||
Bah, it's not that easy, because of bug 1133582 needing to change some of Modern's CSS.
Neil, let's take a step back for a moment. As was mentioned in today's status meeting, the new Toolkit style (obviously adopting some Windows 8.1 design principles) doesn't match the overall style of our themes well. Thus, the question is if we simply want to resolve the breakage and otherwise adopt that style (which personally I don't like either), or if we want to retain the old appearance and thus get independent from whatever Firefox decides the "correct" styles of those pages are.
This would obviously imply forking the respective files of the toolkit/ implementation for suite/ but might be an easy(-ier) solution for bug 1133582, thus making this issue here a WFM once config.xul and config.css are forked.
Keywords: regression
Version: unspecified → Trunk
Comment 7•10 years ago
|
||
Comment on attachment 8564813 [details] [diff] [review]
Proposed patch
I haven't checked, but my guess is that http://mxr.mozilla.org/comm-central/source/suite/themes/classic/mac/communicator/aboutPrivateBrowsing.css needs some love too.
Comment 8•10 years ago
|
||
Comment on attachment 8564813 [details] [diff] [review]
Proposed patch
1. It's still very "Australis" like. The <button> elements have -moz-appearance: none; (coming from .../in-content/common.css). For classic/default we should stick to the native widgets as much as possible.
2. The font size for normalTitle / privateTitle is 2.5em == 37.5px is overly large. I suspect that the Firefox UX people all have retina displays...
The Australis look is too different from the rest of our UI and is rather clashing. Perhaps something like about:neterror ?
Attachment #8564813 -
Flags: review?(philip.chee)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Philip Chee from comment #8)
> The Australis look is too different from the rest of our UI and is rather
> clashing. Perhaps something like about:neterror ?
OK so I was confused by bug 1097111 which wants to change all of Firefox's about:pages; many of them are specific to Firefox so don't affect us; about:neterror appears to be in that category. However there are some that will affect us such as about:about, buildconfig, cache, crashes, license, rights.
Comment 10•10 years ago
|
||
Add about:support to the list, that's why I opened meta bug 1133743 to keep track of those.
Assuming that .../in-content/common.css will be used by all of those pages, would it be a better approach to override that file, given that it essentially overrides all/many xul.css definitions?
Assignee | ||
Comment 11•10 years ago
|
||
We can, although it's a little hacky - the override applies to all themes and someone already complained about our existing override.
Comment 12•10 years ago
|
||
Unfortunatly overrides make life difficult for third party theme authors and we don't want to do that.
Comment 13•10 years ago
|
||
FYI In one of my extensions I used the style directive in chrome.manifest to style about:addons
Assignee | ||
Comment 14•10 years ago
|
||
Just a minor tweak to play nicer with bug 1133582.
Attachment #8564813 -
Attachment is obsolete: true
Attachment #8567306 -
Flags: review?(philip.chee)
Comment 15•10 years ago
|
||
Comment on attachment 8567306 [details] [diff] [review]
Proposed patch
I guess this is the best we can do for the moment. r=me
Attachment #8567306 -
Flags: review?(philip.chee) → review+
Comment 16•10 years ago
|
||
(In reply to Stefan [:stefanh] from comment #7)
> Comment on attachment 8564813 [details] [diff] [review]
> Proposed patch
>
> I haven't checked, but my guess is that
> http://mxr.mozilla.org/comm-central/source/suite/themes/classic/mac/
> communicator/aboutPrivateBrowsing.css needs some love too.
Yeah, I was right - it needs to be changed.
Assignee | ||
Comment 17•10 years ago
|
||
I also removed the now unused warningBoxIcon.
Attachment #8567306 -
Attachment is obsolete: true
Attachment #8573509 -
Flags: review?(stefanh)
Attachment #8573509 -
Flags: review?(philip.chee)
Comment 18•10 years ago
|
||
Comment on attachment 8573509 [details] [diff] [review]
With Mac changes
r=me for the non-mac changes.
Attachment #8573509 -
Flags: review?(philip.chee) → review+
Comment 19•10 years ago
|
||
Comment on attachment 8573509 [details] [diff] [review]
With Mac changes
Thanks for doing this, Neil. r=me for the Mac changes.
Attachment #8573509 -
Flags: review?(stefanh) → review+
Assignee | ||
Comment 20•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.36
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8573509 [details] [diff] [review]
With Mac changes
[Approval Request Comment]
Regression caused by (bug #): 1125636
User impact if declined: Ugliness
Testing completed (on m-c, etc.): Landed on c-c
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None
Attachment #8573509 -
Flags: approval-comm-aurora?
Attachment #8573509 -
Flags: approval-comm-aurora? → approval-comm-aurora+
Assignee | ||
Comment 22•10 years ago
|
||
status-seamonkey2.35:
--- → fixed
status-seamonkey2.36:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•