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)

defect
Not set
normal

Tracking

(seamonkey2.35 fixed, seamonkey2.36 fixed)

RESOLVED FIXED
seamonkey2.36
Tracking Status
seamonkey2.35 --- fixed
seamonkey2.36 --- fixed

People

(Reporter: neil, Assigned: neil)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Bug 1125636 changed the theming of about:config which we were borrowing from.
Attached patch Proposed patch (obsolete) (deleted) — Splinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8564813 - Flags: review?(philip.chee)
Does this break modern? We don't have chrome://global/skin/in-content/info-pages.css there...
Flags: needinfo?(neil)
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.
Bah, it's not that easy, because of bug 1133582 needing to change some of Modern's CSS.
Blocks: 1133743
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 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)
(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.
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?
We can, although it's a little hacky - the override applies to all themes and someone already complained about our existing override.
Unfortunatly overrides make life difficult for third party theme authors and we don't want to do that.
FYI In one of my extensions I used the style directive in chrome.manifest to style about:addons
Attached patch Proposed patch (obsolete) (deleted) — Splinter Review
Just a minor tweak to play nicer with bug 1133582.
Attachment #8564813 - Attachment is obsolete: true
Attachment #8567306 - Flags: review?(philip.chee)
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+
(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.
Attached patch With Mac changes (deleted) — Splinter Review
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 on attachment 8573509 [details] [diff] [review] With Mac changes r=me for the non-mac changes.
Attachment #8573509 - Flags: review?(philip.chee) → review+
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+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.36
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+
Blocks: 1192276
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: