388030 - Clicking RSS causes the location bar to lose Domain Highlight on use of 'Back'

Status: VERIFIED WONTFIX

(Firefox :: Address Bar, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/2007071305 Minefield/3.0a7pre Firefox/3.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/2007071305 Minefield/3.0a7pre Firefox/3.0

Go to the URL www.cnn.com or www.msnbc.com and click on the RSS Icon.  These sites offer a multiple choice of feeds.  Click either and note the Domain Highlighting goes away.  Clicking Back then results in the location bar no longer displaying the Domain Highlight across all tabs.  

Clicking on the location bar and then in the webpage body restores the Domain Highlighting.  

Web pages that offer only a single link when clicking the RSS feed does not seem to break the Domain Highlight.  

Same occurs if you hover over the location bar and let it turn all black, then drag the favicon to the bookmarks bar, the Domain Highlighting is lost.  

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Actual Results:  
Domain Hightlighting disappears leaving the user without the feature.

Expected Results:  
Domain Hightlighting should not disappear leaving the user without the feature.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a7pre) Gecko/2007071305 Minefield/3.0a7pre Firefox/3.0 ID:2007071305

Comment 1

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a7pre) Gecko/2007071305 Minefield/3.0a7pre ID:2007071305
I see this too --> NEW

Comment 2

[Dão Gottwald [:dao]]

This is because the mouseout event isn't dispatched if you open the Feed icon's popup menu.

Comment 3

[Dão Gottwald [:dao]]

Attachment: patch

This avoids the whole problem by switching to the styled view as soon as you move the mouse over the Feed icon (and others).

Note that |this._plain = false;| in the current code doesn't do anything. That has to be |this._formatted = true;|. Also, I'm now setting that directly in _prettyView instead of _initPrettyView. Otherwise |if (!this.plain && !this._isMouseOverAddress(event))| would malfunction when moving the mouse over the favicon.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch v2

mouseout was not working as expected

Comment 5

[Dão Gottwald [:dao]]

Attachment: patch v3

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch v3.1

Oops, v3 was nonsense.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Does this affect the location bar pretty-print display for characters and spaces as well, or just the domain highlighting?

Comment 8

[Dão Gottwald [:dao]]

This affects the pretty-print display in any case. (Domain highlighting was just one way to style it.)

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Then yes, it blocks :)

Comment 10

[Dão Gottwald [:dao]]

Attachment: patch v4

_isMouseOverElement removed

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch v4.1

there's no good reason to use _inputBox instead of inputField

Comment 12

[Dão Gottwald [:dao]]

Attachment: patch v4.2

indention fixed

Comment 13

[Tony Chung [:tchung]]

Can you please update the new target milestone?  Thanks.

Comment 14

[Dão Gottwald [:dao]]

Attachment: patch v4.3

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Attachment: screenshot of bug

The latest patch still has trouble dealing with long URLs and small windows.

Comment 16

[Dão Gottwald [:dao]]

Attachment: patch v4.4

nasty ... but not as nasty as flex="9999"

Comment 17

[Dão Gottwald [:dao]]

width="1" is needed for very long URLs like http://tinyurl.com/3ykhdb

Comment 18

[:Gavin Sharp [email: [e-mail]]]

I still see the bug from my screenshot (attachment 282692 [details]) with the latest patch.

Comment 19

[Dão Gottwald [:dao]]

Hm, I thought I didn't. Will recheck.

Comment 20

[Dão Gottwald [:dao]]

Attachment: screenshot without bug

Here's what I see. I've obviously modified the urlbar in other ways, but I don't think that should make a difference. I will try a clean build + profile.

Note that I did see the bug with the previous patch.

Comment 21

[Dão Gottwald [:dao]]

Comment on attachment 282706 [details] [diff] [review]
patch v4.4

Ok, seeing it now. It depends on whether the protocol is hidden.

Comment 22

[Dão Gottwald [:dao]]

Attachment: patch v4.5

Comment 23

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 24

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 283033 [details] [diff] [review]
patch v4.5

>Index: browser/base/content/urlbarBindings.xml

>       <method name="_initPrettyView">

>+          this._formattedURL.hidden = false;
>+          this._prePath.maxWidth =
>+            this._prePath.width = "";
>+          this._prePath.flex = 0;
>+          this._prePath.maxWidth =
>             this._prePath.width = this._prePath.boxObject.width;
>+          this._prePath.flex = 1;

This is the hack used to get the sizing right, presumably? Could you add a comment explaining it?

>Index: browser/themes/pinstripe/browser/browser.css
>Index: browser/themes/winstripe/browser/browser.css

I wonder if we want to increase the url bar's min-width given the way this looks with small windows. Having the host just end abruptly without a "..." could potentially be a spoofing risk. Followup bug?

Comment 25

[Dão Gottwald [:dao]]

(In reply to comment #24)
> This is the hack used to get the sizing right, presumably? Could you add a
> comment explaining it?

I'll add one. (I realize that my code could use more comments in general.)

> I wonder if we want to increase the url bar's min-width given the way this
> looks with small windows. Having the host just end abruptly without a "..."
> could potentially be a spoofing risk. Followup bug?

I don't think |crop| can be used there, and increasing the minimum width will cause the search bar to be pushed off the window earlier, esp. with Larry. But it's worth tracking anyway, so yes, I'll file a bug later.

Comment 26

[Dão Gottwald [:dao]]

Attachment: patch v4.5 with comment

Comment 27

[Dão Gottwald [:dao]]

thanks for the review, Gavin.

Comment 28

[Reed Loden [:reed]]

Checking in browser/base/content/urlbarBindings.xml;
/cvsroot/mozilla/browser/base/content/urlbarBindings.xml,v  <--  urlbarBindings.xml
new revision: 1.34; previous revision: 1.33
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.85; previous revision: 1.84
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.105; previous revision: 1.104
done

Comment 29

[Reed Loden [:reed]]

This regressed perf on mac only.

Before:
Tp:148ms
Tp2:101.575ms
Tdhtml:523ms
Txul:149ms
Ts:851ms

After:
Tp:165ms
Tp2:105.5875ms
Tdhtml:523ms
Txul:155ms
Ts:871ms

I didn't back it out yet, but I will tomorrow if the fix isn't obvious.

Comment 30

[Dão Gottwald [:dao]]

Unless |crop| is much more expensive than the scrollbox was (on Mac only?!) it could only be this:

-          this._plain = false;
           this._protocol.hidden = false;
-          this._presentationBox.hidden = false;
-          this._prePath.width = "";
-          if (this._protocolHidden) {
+          this._formattedURL.hidden = false;
+          this._prePath.maxWidth =
+            this._prePath.width = "";
+          this._prePath.flex = 0;
+
+          this._prePath.maxWidth =
             this._prePath.width = this._prePath.boxObject.width;
+          this._prePath.flex = 1;
+
+          if (this._protocolHidden)
             this._protocol.hidden = true;
-          }

Because I expect boxObject.width to trigger layout. But an 11% Tp regression? And again, Mac only? That's ... interesting, to say the least.

Comment 31

[Dão Gottwald [:dao]]

Attachment: prePathCache

This would help if the location changes while staying on the same host. However, I don't see how this could save 17ms on pageload. And I assume that Tp is about pages from different hosts anyway ...

Comment 32

[Dão Gottwald [:dao]]

Latest numbers:

Tp:143ms
Tp2:96.2ms
Tdhtml:398ms
Txul:130ms
Ts:853ms

Not sure what's going on there.

Comment 33

[Reed Loden [:reed]]

Backed out due to mac-only perf regression.

Comment 34

[Dão Gottwald [:dao]]

Attachment: patch v5

With the change from comment 31 and overflow:-moz-hidden-unscrollable instead of overflow:hidden.

I don't really know where the slowdown on Mac originated, any hint is welcome.

Comment 35

[Dão Gottwald [:dao]]

Attachment: patch v5.1

Thanks to overflow:-moz-hidden-unscrollable, I can revert _initPrettyView to something simpler if the protocol isn't hidden.

Comment 36

[juan becerra [:juanb]]

Litmus triage team: test case to be added in Litmus by Tomcat

Comment 37

[Dão Gottwald [:dao]]

Attachment: less radical patch

Comment 38

[u88484]

Dao, could you give the reasoning behind resolving this WONTFIX?

Comment 39

[Peter van der Woude [:Peter6]]

i assume this was marked wontfix by accident, reopening

Comment 40

[Dão Gottwald [:dao]]

There won't be a patch landing here, as there won't be a 'Domain Highlight'.

Comment 41

[Peter van der Woude [:Peter6]]

ok,sorry for the span than, resetting wontfix

Comment 42

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified