Closed Bug 1178008 Opened 5 years ago Closed 5 years ago

The Private Browsing window is messed up when using certain third party themes. They need to include or update aboutPrivateBrowsing.css

Categories

(SeaMonkey :: Themes, defect)

SeaMonkey 2.35 Branch
defect
Not set
normal

Tracking

(seamonkey2.35 verified, seamonkey2.36 fixed, seamonkey2.37 fixed, seamonkey2.38 fixed, seamonkey2.39 fixed)

RESOLVED FIXED
seamonkey2.39
Tracking Status
seamonkey2.35 --- verified
seamonkey2.36 --- fixed
seamonkey2.37 --- fixed
seamonkey2.38 --- fixed
seamonkey2.39 --- fixed

People

(Reporter: rn10950, Assigned: rn10950)

References

Details

User Story

http://hg.mozilla.org/comm-central/rev/8f50d091d957          SeaMonkey 2.39
http://hg.mozilla.org/releases/comm-aurora/rev/a38bd6d34748  SeaMonkey 2.38
http://hg.mozilla.org/releases/comm-beta/rev/659cd4f825fd    SeaMonkey 2.37
http://hg.mozilla.org/releases/comm-release/rev/11dc25637a4f SeaMonkey 2.36
http://hg.mozilla.org/releases/comm-release/rev/e70cf3ae7723 SeaMonkey 2.35

Attachments

(2 files, 3 obsolete files)

The new private browsing window message has confusing text on Seamonkey 2.35 when using a custom theme, if the theme does not include aboutPrivateBrowsing.css. I have verified this bug on Windows 2003 x86 and Windows 7 x64.

Theme Developers: Include aboutPrivateBrowsing.css into the chrome/communicator/ directory of your theme.

aboutPrivateBrowsing.css: http://mxr.mozilla.org/comm-central/source/suite/themes/classic/communicator/aboutPrivateBrowsing.css
Summary: The Private Browsing window has a confusing message when using a custom theme → The Private Browsing window is messed up when using certain third party themes. They need to include or update aboutPrivateBrowsing.css
Can you elaborate which text is shown and why? Also, is it a problem with the about:privatebrowsing page as such, or is it just a matter of 3rd-party themes needing to catch up with recent changes?

See bug 1133380 and bug 1133743 for reference.
This example uses the Orthodox theme located at https://addons.mozilla.org/en-us/seamonkey/addon/orthodox-for-seamonkey/. If I drop aboutPrivateBrowsing.css into the chrome/communicator/ directory in the extension, the bug goes away.
Comment on attachment 8626950 [details]
example using the "Orthodox" theme

I like that theme.  :-)

> #warningBox:not(.private) .private,
> #warningBox:not(.normal) .normal {
>   display: none;
> }

This is apparently the missing rule, but that's weird, because it has been around before bug 1133380 already (the new stuff include the "title" classes and rearranging config.css in Toolkit).

Neil, Ratty: any idea what's going wrong here?
Flags: needinfo?(philip.chee)
Flags: needinfo?(neil)
(In reply to rn10950 from comment #2)
> This example uses the Orthodox theme located at
> https://addons.mozilla.org/en-us/seamonkey/addon/orthodox-for-seamonkey/. If
> I drop aboutPrivateBrowsing.css into the chrome/communicator/ directory in
> the extension, the bug goes away.

communicator/aboutPrivateBrowsing.css was added way back by bug 842439. If about:privatebrowsing happened to display correctly without it then that was just luck rather than design. Or have I misunderstood what you mean by that?
(In reply to neil@parkwaycc.co.uk from comment #4)
> (In reply to rn10950 from comment #2)
> > This example uses the Orthodox theme located at
> > https://addons.mozilla.org/en-us/seamonkey/addon/orthodox-for-seamonkey/. If
> > I drop aboutPrivateBrowsing.css into the chrome/communicator/ directory in
> > the extension, the bug goes away.
> 
> communicator/aboutPrivateBrowsing.css was added way back by bug 842439. If
> about:privatebrowsing happened to display correctly without it then that was
> just luck rather than design. Or have I misunderstood what you mean by that?

about:privatebrowsing displays fine when using the default or modern themes, but it doesn't display right when using a third-party theme that doesn't already include aboutPrivateBrowsing.css. Once I added aboutPrivateBrowsing.css into the communicator/ directory of the third-party extension, it fixes itself.
Did it work with SM 2.25 thru 2.33.1?

This sounds more like an error in the theme to me for not providing that rule to start with. There is also no Toolkit version of that file which might have accidentally been grabbed instead.
I just checked both 2.33.1 and 2.25, and you're right, it also affects 2.25-2.33.1. It's just less obvious, as there is no big yellow warning icon in 2.25-2.33.1. One thing that I do have to note is, that if I drop the current aboutPrivateBrowsing.css from MXR into 2.25, it creates a checkerboard background of the blue "i" icon. (http://i.imgur.com/NfdOs46.png) I definitely think that we should provide the current aboutPrivateBrowsing.css (and other toolkit CSS files)to the extensions by default, while providing the third-party theme developers to override it with their own if they choose to, in order to preserve backwards-compatibility for older themes where the developers have discontinued them or forgotten to include it.
I'm wondering if the code mentioned in comment #3 should be moved into suite/browser/navigator.css given that it's rather function than style.

(In reply to rn10950 from comment #7)
> I definitely think that we should provide the current aboutPrivateBrowsing.css
> (and other toolkit CSS files) to the extensions by default,

As said, aboutPrivateBrowsing.css isn't Toolkit code, thus there isn't really any default fallback (furthermore, I have no clue what jar.mn magic would be involved to make that happen).
(In reply to rsx11m from comment #8)
> I'm wondering if the code mentioned in comment #3 should be moved into
> suite/browser/navigator.css given that it's rather function than style.
> 
> (In reply to rn10950 from comment #7)
> > I definitely think that we should provide the current aboutPrivateBrowsing.css
> > (and other toolkit CSS files) to the extensions by default,
> 
> As said, aboutPrivateBrowsing.css isn't Toolkit code, thus there isn't
> really any default fallback (furthermore, I have no clue what jar.mn magic
> would be involved to make that happen).

On my local copy of comm-central, I modified suite/browser/navigator.css and I am building when I go to sleep. (I had to clobber) If it works, I will make and upload a patch.
The tricky thing is to define that rule to apply to about:privatebrowsing only. Another option is to place a theme-independent aboutPrivateBrowsing.css next to aboutPrivateBrowsing.xul, thus avoiding possible regression of other pages which may be using a "normal" or "private" class, at the cost of adding yet another file to jar.mn.
(In reply to rsx11m from comment #10)
> The tricky thing is to define that rule to apply to about:privatebrowsing
> only. Another option is to place a theme-independent
> aboutPrivateBrowsing.css next to aboutPrivateBrowsing.xul, thus avoiding
> possible regression of other pages which may be using a "normal" or
> "private" class, at the cost of adding yet another file to jar.mn.

Yes, that would be the way to go, similar to certError.css.
Flags: needinfo?(neil)
(In reply to neil@parkwaycc.co.uk from comment #11)
> (In reply to rsx11m from comment #10)
> > The tricky thing is to define that rule to apply to about:privatebrowsing
> > only. Another option is to place a theme-independent
> > aboutPrivateBrowsing.css next to aboutPrivateBrowsing.xul, thus avoiding
> > possible regression of other pages which may be using a "normal" or
> > "private" class, at the cost of adding yet another file to jar.mn.
> 
> Yes, that would be the way to go, similar to certError.css.

I created a new CSS file in suite/common (jar.mn) and it works fine. Now I just have to work out a slight CSS bug where the bootom of the exclamation icon gets cut off.
This patch adds an aboutPrivateBrowsing.css into suite/common/ that works.
Attachment #8627540 - Flags: review?(neil)
Attachment #8627540 - Attachment is obsolete: true
Attachment #8627540 - Flags: review?(neil)
Attachment #8627542 - Flags: review?(neil)
Comment on attachment 8627542 [details] [diff] [review]
basically the same as before, just fixed the ordering in jar.mn to be alphabetical.

You should remove the now duplicated rules from the theme versions of the CSS.
Attachment #8627542 - Attachment is obsolete: true
Attachment #8627542 - Flags: review?(neil)
Attachment #8627577 - Flags: review?(neil)
Flags: needinfo?(philip.chee)
>+++ b/suite/themes/classic/communicator/aboutPrivateBrowsing.css
>+++ b/suite/themes/modern/communicator/aboutPrivateBrowsing.css

There is also suite/themes/classic/mac/communicator/aboutPrivateBrowsing.css (frequently forgotten).

>+++ b/suite/common/aboutPrivateBrowsing.css
>+.title {
>+  background-size: auto;
>+}

This rule wasn't in modern's aboutPrivateBrowsing.css, any impact on the layout?
Assignee: nobody → rn10950
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to rsx11m from comment #17)
> >+++ b/suite/themes/classic/communicator/aboutPrivateBrowsing.css
> >+++ b/suite/themes/modern/communicator/aboutPrivateBrowsing.css
> 
> There is also suite/themes/classic/mac/communicator/aboutPrivateBrowsing.css
> (frequently forgotten).
> 

I will remove this, but I do not have a Mac to test it with (at least one powerful enough to build SeaMonkey)

> >+++ b/suite/common/aboutPrivateBrowsing.css
> >+.title {
> >+  background-size: auto;
> >+}
> 
> This rule wasn't in modern's aboutPrivateBrowsing.css, any impact on the
> layout?

I tested in the Modern theme, and I did not notice any difference.
Attachment #8627577 - Attachment is obsolete: true
Attachment #8627577 - Flags: review?(neil)
Attachment #8627821 - Flags: review?(neil)
Comment on attachment 8627821 [details] [diff] [review]
removed the Mac rules

>+#warningBox:not(.private) .private,
>+#warningBox:not(.normal) .normal {
>+  display: none;
>+}
>+.title {
>+  background-size: auto;
>+}
Nit: blank line between styles (as per the places from which you deleted them).
Attachment #8627821 - Flags: review?(neil) → review+
I fixed the nits before pushing to comm-central:
http://hg.mozilla.org/comm-central/rev/8f50d091d957
Target Milestone: --- → seamonkey2.39
Comment on attachment 8627821 [details] [diff] [review]
removed the Mac rules

[Approval Request Comment]
Regression caused by (bug #): Bug 1133380
User impact if declined: The Private Browsing window is messed up when using certain third party themes.
Testing completed (on m-c, etc.): Yes 
Risk to taking this patch (and alternatives if risky): none bug fix.
String changes made by this patch: none.
Attachment #8627821 - Flags: approval-comm-release?
Attachment #8627821 - Flags: approval-comm-beta?
Attachment #8627821 - Flags: approval-comm-aurora?
Attachment #8627821 - Flags: approval-comm-release?
Attachment #8627821 - Flags: approval-comm-release+
Attachment #8627821 - Flags: approval-comm-beta?
Attachment #8627821 - Flags: approval-comm-beta+
Attachment #8627821 - Flags: approval-comm-aurora?
Attachment #8627821 - Flags: approval-comm-aurora+
Keywords: checkin-needed
Whiteboard: [c-n: branches, including SEAMONKEY_2_35_RELEASE_BRANCH]
http://hg.mozilla.org/comm-central/rev/8f50d091d957          SeaMonkey 2.39
http://hg.mozilla.org/releases/comm-aurora/rev/a38bd6d34748  SeaMonkey 2.38
http://hg.mozilla.org/releases/comm-beta/rev/659cd4f825fd    SeaMonkey 2.37
http://hg.mozilla.org/releases/comm-release/rev/11dc25637a4f SeaMonkey 2.36
http://hg.mozilla.org/releases/comm-release/rev/e70cf3ae7723 SeaMonkey 2.35
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
User Story: (updated)
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: branches, including SEAMONKEY_2_35_RELEASE_BRANCH]
You need to log in before you can comment on or make changes to this bug.