Closed Bug 335113 Opened 15 years ago Closed 14 years ago

More obvious secure site indication for SeaMonkey - yellow URL bar

Categories

(SeaMonkey :: UI Design, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zug_treno, Unassigned)

References

()

Details

(Keywords: fixed-seamonkey1.1.1, fixed-seamonkey1.1a, relnote, Whiteboard: Mac classic fixed for seamonkey1.1.1 relnote-seamonkey1.1.1)

Attachments

(7 files, 7 obsolete files)

1.62 KB, image/gif
Details
85 bytes, image/gif
Details
85 bytes, image/gif
Details
1.94 KB, image/gif
Details
1.87 KB, patch
jag+mozilla
: review+
Details | Diff | Splinter Review
7.40 KB, image/gif
Details
1000 bytes, patch
neil
: review+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060404 SeaMonkey/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060404 SeaMonkey/1.0.1

Firefox sets the background of the URL/location bar to yellow when you visit a secure/https site (see Firefox bug 244025). This might be nice for SeaMonkey as well.

Reproducible: Always

Steps to Reproduce:
1.Go to a secure site.
2.Look at the URL bar.
Actual Results:  
The background of the URL bar is still white.

Expected Results:  
The background of the URL bar changes to yellow.
This is a 'Copy and Paste'-patch made from the patches in Firefox bug 244025 and Firefox bug 249680. I just copied the relevant URL bar parts of these patches, I didn't change the statusbar/lock icon code. This patch seems to work with both Classic and Modern.
Attachment #219460 - Flags: superreview?
Attachment #219460 - Flags: review?(db48x)
Comment on attachment 219460 [details] [diff] [review]
Diff made with Patch Maker and SeaMonkey 1.5a/20060422

move the css rule into the themes
Attachment #219460 - Flags: review?(db48x) → review-
Improved patch, I moved the css rule into the Classic and Modern navigator.css files.
Attachment #219460 - Attachment is obsolete: true
Attachment #223445 - Flags: review?(db48x)
Attachment #219460 - Flags: superreview?
Comment on attachment 223445 [details] [diff] [review]
Second diff made with Patch Maker and SeaMonkey 1.5a/20060525

looks good. r=db48x
Attachment #223445 - Flags: review?(db48x) → review+
Attachment #223445 - Flags: superreview?(neil)
Comment on attachment 223445 [details] [diff] [review]
Second diff made with Patch Maker and SeaMonkey 1.5a/20060525

>--- classic\skin\classic\navigator\navigator.css.bak	2005-05-01 02:48:34.000000000 +0200
>+++ classic\skin\classic\navigator\navigator.css	2006-05-26 16:38:55.562500000 +0200
>@@ -195,6 +195,13 @@
>   -moz-border-left-colors: transparent ThreeDShadow ThreeDDarkShadow;
> }
> 
>+#urlbar[level="high"] > .autocomplete-textbox-container,
>+#urlbar[level="low"] > .autocomplete-textbox-container,
>+#urlbar[level="broken"] > .autocomplete-textbox-container {
>+  background-color: #F5F6BE; /* #F7F898; */
>+  color: #000000;
>+}
The Classic theme should use system colours where available.
I suggest the tooltip colours would be a reasonable choice.

>+#urlbar[level="high"] > .autocomplete-textbox-container,
>+#urlbar[level="low"] > .autocomplete-textbox-container,
>+#urlbar[level="broken"] > .autocomplete-textbox-container {
>+  background-color: #F5F6BE; /* #F7F898; */
>+  color: #000000;
>+}
The Modern urlbar is styled differently. In particular the dropmarker is partially transparent, so its background needs to change too. However you should not need to change the text colour. I am also not convinced of the choice of colour; by comparison the locked icon uses the colour #E8DB99.
Attachment #223445 - Flags: superreview?(neil) → superreview-
I changed the Classic colors to InfoBackground and InfoText. The Modern background-color is now #E8DB99, as is the dropmarker background.
Attachment #223445 - Attachment is obsolete: true
Attachment #224311 - Flags: superreview?
Attachment #224311 - Flags: review?(db48x)
Attachment #224311 - Flags: superreview? → superreview?(neil)
Attachment #224311 - Attachment description: Third diff made with with Patch Maker and SeaMonkey 1.5a/20060602 → Third diff made with Patch Maker and SeaMonkey 1.5a/20060602
(In reply to comment #5)
> The Classic theme should use system colours where available.
> I suggest the tooltip colours would be a reasonable choice.

I'd suggest tooltip color or a firefox (winstripe) color.

> The Modern urlbar is styled differently. In particular the dropmarker is
> partially transparent, so its background needs to change too. However you
> should not need to change the text colour. I am also not convinced of the
> choice of colour; by comparison the locked icon uses the colour #E8DB99.

That's an ugly color on its own, bug #E8DB99 does seem like it would fit well in Modern.
Attached image Modern screenshot
Classic looks fine, but Modern still looks wrong.

Note: you should be able to use #urlbar[level]
Screenshot with the white parts of ubhist-arrow.gif/ubhist-arrow-act.gif made transparent, would this be better?
Sorry for being unclear, I should have said when attaching the screenshot that I was referring to the background which still shows as a strange-looking border.
New patch using #urlbar[level] in both Classic and Modern, no more white background/border visible in Modern.
Attachment #224311 - Attachment is obsolete: true
Attachment #224362 - Flags: superreview?(neil)
Attachment #224362 - Flags: review?(db48x)
Attachment #224311 - Flags: superreview?(neil)
Attachment #224311 - Flags: review?(db48x)
Attachment #224346 - Attachment is obsolete: true
Ok, maybe I'm missing something, but why are we displaying the yellow (secure) background colour for level=broken? I admit that the red from lock-broken.gif is perhaps a bit ugly to put up there, but could we then just leave it white instead?
Comment on attachment 224362 [details] [diff] [review]
Fourth diff made with Patch Maker and SeaMonkey 1.5a/20060603

OK, after some consideration, I agree with jag here, so we'll need to specify the two "good" levels. Sorry for messing you around like that.

(In reply to comment #15)
>why are we displaying the yellow (secure) background colour for level=broken?
To clarify, "broken" means mixed content (typically advert from insecure site, but also unfortunately for remote XUL on secure websites).
Attachment #224362 - Flags: superreview?(neil) → superreview-
Attached patch Edited 4th diff (obsolete) — Splinter Review
(In reply to comment #15)
> I admit that the red from lock-broken.gif
> is perhaps a bit ugly to put up there, but could we then just leave it white
> instead?

Just changing 'this.urlBar.setAttribute("level", "broken");' to 'this.urlBar.removeAttribute("level");' in nsBrowserStatusHandler.js gives me a broken lock icon and a white URL bar background.

(In reply to comment #16)
> OK, after some consideration, I agree with jag here, so we'll need to specify
> the two "good" levels.

Any suggestions for the Classic and Modern colors?
Attachment #224362 - Attachment is obsolete: true
Attachment #224362 - Flags: review?(db48x)
(In reply to comment #17)
>(In reply to comment #16)
>>OK, after some consideration, I agree with jag here, so we'll need to specify
>>the two "good" levels.
>Any suggestions for the Classic and Modern colors?
You misunderstood me (don't worry, you're not the first to do that).
I meant that the URLbar should not change for mixed content.

Note: the mixed content notification happens after a secure content notification, so that the URLbar will briefly flash...

Comment on attachment 224448 [details] [diff] [review]
Edited 4th diff

The question is, should we still add the broken attribute to the URLbar so that other themes have the chance to use their own styling?
With this patch the URL bar background changes back to white on a broken/mixed page and I readded the 'broken' attribute for other themes to use.
Attachment #224448 - Attachment is obsolete: true
Attachment #225990 - Flags: superreview?(neil)
Attachment #225990 - Flags: review+
Comment on attachment 225990 [details] [diff] [review]
Fifth diff made with Patch Maker and SeaMonkey 1.5a/20060615

Sorry for the delay, I was overwhelmed with work this week.
Attachment #225990 - Flags: superreview?(neil) → superreview+
Daniel, Neil and Peter thanks for the (super)reviews. Is this patch ready for checkin now, and if so who could do that?
Fix checked in on the trunk.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Verified FIXED using both Classic and Modern skins on SeaMonkey trunk build 2006-07-02-09, Windows XP.
Status: RESOLVED → VERIFIED
Any chance of having a global "Appearance" override preference for this? One that disables the theme css change all together, either in about:config or Preferences->Appearance->Colors?
SeaMonkey 1.1 final is supposed to have this according to the release notes. I still only see the closed lock at the bottom, no color change in the address bar.  Mac OS X 10.3.9, Classic Theme.
(In reply to comment #27)
> SeaMonkey 1.1 final is supposed to have this according to the release notes. I
> still only see the closed lock at the bottom, no color change in the address
> bar.  Mac OS X 10.3.9, Classic Theme.
> 

Hmm, I guess I thought this was a windows-only fix. But now I see that the OS field is "All".

+  background-color: InfoBackground;

I don't think "InfoBackground" is useful on mac for this purpose, it seems to return white... In the bugs mentioned in comment #1 you'll notice that Pinstripe is treated differently, like this:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/browser/themes/pinstripe/browser&command=DIFF_FRAMESET&file=browser.css&rev1=1.3&rev2=1.4&root=/cvsroot

You'll probably need to hard-code the bgcolor for everyone if you can't put the style rules for mac in a mac-only file.

It should probably be mentioned in the release notes that it only works for win/nix(?).
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Just to clarify:

It works in Modern on mac, but not with Classic.
Thanks for clarifying. Confirmed working in Modern.
Neil, are you OK with a hard-coded background-color in navigator.css? Or shall we override the style rules in navigator.css from some mac platform-specific file in Classic?

In the long term, the native theme code should of course be corrected. But I don't see any other short-term option than hard-coding the background-color somewhere in the theme files since InfoBackground is used in Pinstripe elsewhere (and I can imagine that they don't want the color change).

Fwiw, the mac tooltip bgcolor that you get with "-moz-appearance: tooltip;" on trunk is #FFFFC7 (hardcoded in nsNativeThemeCocoa).

(In reply to comment #26)
> Any chance of having a global "Appearance" override preference for this? One
> that disables the theme css change all together, either in about:config or
> Preferences->Appearance->Colors?

You should be able to override the "secure" background-color with your own preferred color by adding those lines to your userChrome.css file:

#urlbar[level="high"] > .autocomplete-textbox-container,
#urlbar[level="low"] > .autocomplete-textbox-container {
  background-color: yourColor !important;

(In reply to comment #31)
>Neil, are you OK with a hard-coded background-color in navigator.css?
No, but then I didn't know that Mac gets this badly wrong again (is there really no system colour set on the Mac?)

>Or shall we override the style rules in navigator.css from some mac
>platform-specific file in Classic?
I guess that's the best we can do on the branch.

>In the long term, the native theme code should of course be corrected.
I'll let you file the appropriate bug :-)

>Fwiw, the mac tooltip bgcolor that you get with "-moz-appearance: tooltip;" on
>trunk is #FFFFC7 (hardcoded in nsNativeThemeCocoa).
Is #FFFFC7 noticeably yellow? I don't think we can use -moz-appearance in this case, as it overrides the CSS padding.

>(In reply to comment #26)
>>Any chance of having a global "Appearance" override preference for this? One
>>that disables the theme css change all together, either in about:config or
>>Preferences->Appearance->Colors?
>You should be able to override the "secure" background-color with your own
>preferred color by adding those lines to your userChrome.css file:
You should also be able to override the colour used by tooltips (and thus the "secure" background) be setting the ui.infobackground preference.
> Is #FFFFC7 noticeably yellow? I don't think we can use -moz-appearance in this
> case, as it overrides the CSS padding.

If you want to use the tooltip background to indicate a secure url that is the color we'll get for mac once the native theme code is fixed ;-)
Comment on attachment 252215 [details]
Screenshot of URL bar with 

That's nice! I'd actually overlooked that Windows defaults to the paler #FFFFE1.
I can only think of two files in classic/global/mac where one could put the style rule: autocomplete.css or textbox.css. It feels a bit awkward, though. You know that I want a mac navigator.css file ;-) (at least on trunk, please)

Anyway, which one do you prefer?



autocomplete.css is slightly less inaccurate.
(In reply to comment #32)
> >(In reply to comment #26)
> >>Any chance of having a global "Appearance" override preference for this? One
> >>that disables the theme css change all together, either in about:config or
> >>Preferences->Appearance->Colors?
> >You should be able to override the "secure" background-color with your own
> >preferred color by adding those lines to your userChrome.css file:
> You should also be able to override the colour used by tooltips (and thus the
> "secure" background) be setting the ui.infobackground preference.

Thank you for the tips. I have already overridden it in my userChrome.css file with some help from nice people in #seamonkey.

I was thinking something a little less hackish, something that I can tell my friends? Also, I would prefer if there were a way to keep yellow-background tooltips but not have the color-changing urlbar. Perhaps an tickbox in Appearance or Security? (Ticked by default) Or something in about:config.
Attachment #252227 - Flags: superreview?(neil)
Attachment #252227 - Flags: review?(neil)
Comment on attachment 252227 [details] [diff] [review]
Override style rule in navigator.css

Not that I can actually test this, you understand ;-)
Attachment #252227 - Flags: superreview?(neil)
Attachment #252227 - Flags: superreview+
Attachment #252227 - Flags: review?(neil)
Attachment #252227 - Flags: review+
Comment on attachment 252227 [details] [diff] [review]
Override style rule in navigator.css

This will make the "secure"  url bgcolor work on mac classic.
Attachment #252227 - Flags: approval-seamonkey1.1.1?
Depends on: 367672
Comment on attachment 252227 [details] [diff] [review]
Override style rule in navigator.css

first-a=me for 1.1.1, one still needed to go
no change in color on an https system on my copy of 1.1

Thanks
Richard
Attachment #252290 - Flags: review+
Comment on attachment 252290 [details]
RE:color change for HTTPS sites in URL bar

It is not appropriate to attach (Windows) binaries of unknown origin. If you have trouble with a specific build, just name its location on an official Mozilla server.
It is also not appropriate to flag such binaries as reviewed, which only gives the impression that some scam is attempted (I didn't test this binary, though)...
Attachment #252290 - Attachment is obsolete: true
Attachment #252290 - Flags: review+
Comment on attachment 252227 [details] [diff] [review]
Override style rule in navigator.css

first-a=me.  Get a second approval.
(In reply to comment #44)
> (From update of attachment 252227 [details] [diff] [review])
> first-a=me.  Get a second approval.

Your approval is the second one. See comment #41 ;-)
Attachment #252227 - Flags: approval-seamonkey1.1.1? → approval-seamonkey1.1.1+
Comment on attachment 252227 [details] [diff] [review]
Override style rule in navigator.css

Landed on MOZILLA_1_8_BRANCH.
This is now fixed for Mac classic in 1.1.1 and trunk (bug 367672 fixed it on trunk).
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: Mac classic fixed for seamonkey1.1.1
Whiteboard: Mac classic fixed for seamonkey1.1.1 → Mac classic fixed for seamonkey1.1.1 relnote-seamonkey1.1.1
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.