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

RESOLVED FIXED in seamonkey2.39

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rn10950, Assigned: rn10950)

Tracking

SeaMonkey 2.35 Branch
seamonkey2.39
Dependency tree / graph

SeaMonkey Tracking Flags

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

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 attachments, 3 obsolete attachments)

Assignee

Description

4 years ago
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

Updated

4 years ago
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

Comment 1

4 years ago
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.
Assignee

Comment 2

4 years ago
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 3

4 years ago
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?
Assignee

Comment 5

4 years ago
(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.

Comment 6

4 years ago
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.
Assignee

Comment 7

4 years ago
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.

Comment 8

4 years ago
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).
Assignee

Comment 9

4 years ago
(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.

Comment 10

4 years ago
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)
Assignee

Comment 12

4 years ago
(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.
Assignee

Comment 13

4 years ago
This patch adds an aboutPrivateBrowsing.css into suite/common/ that works.
Attachment #8627540 - Flags: review?(neil)
Assignee

Comment 14

4 years ago
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.
Assignee

Comment 16

4 years ago
Attachment #8627542 - Attachment is obsolete: true
Attachment #8627542 - Flags: review?(neil)
Attachment #8627577 - Flags: review?(neil)

Updated

4 years ago
Flags: needinfo?(philip.chee)

Comment 17

4 years ago
>+++ 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
Assignee

Comment 18

4 years ago
(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.
Assignee

Comment 19

4 years ago
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+

Comment 21

4 years ago
I fixed the nits before pushing to comm-central:
http://hg.mozilla.org/comm-central/rev/8f50d091d957
Target Milestone: --- → seamonkey2.39

Comment 22

4 years ago
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?

Updated

4 years ago

Updated

4 years ago
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+

Updated

4 years ago
Keywords: checkin-needed
Whiteboard: [c-n: branches, including SEAMONKEY_2_35_RELEASE_BRANCH]

Comment 23

4 years ago
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: 4 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.