631812 - URLBar broken on certain themes

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Pardal Freudenthal (:ShareBird)]

Attachment: Patch bringing back containers for the urlbar stuff

Bug 541656 - Display hyperlink URLs at bottom of window (instead of right side of location bar) - has broken my themes in a very difficult way to fix. This could also affect other themes that are using the dropped classes urlbar-frontcap-and-textbox and urlbar-frontcap to skin the urlbar...

I'm proposing to bring back the above mentioned containers; I guess it is a no-risk modification and will simplify a lot the work on the themes, since we are so close to the FF 4.0 release.

Comment 1

[Dão Gottwald [:dao]]

Judging by <http://mxr.mozilla.org/mozilla-central/source/browser/base/content/urlbarBindings.xml> and <http://mxr.mozilla.org/mozilla1.9.2/source/toolkit/content/widgets/autocomplete.xml>, the current binding is compatible with as it was in Firefox 3.6, except that autocomplete-icon seems to have moved -- we should probably fix that.
So if your theme supports Firefox 3.6, this should be super-easy to fix, and the extra containers are more likely to make life harder for themes that aren't updated for Firefox 4 yet by breaking child selectors.

Comment 2

[Dão Gottwald [:dao]]

(In reply to comment #1)
> the current binding is compatible with as it was in Firefox 3.6, except that
> autocomplete-icon seems to have moved -- we should probably fix that.

More precisely, the insertion point for the <box> nodes has moved. autocomplete-icon itself isn't used.

I'll check if relocating this would break something in the default themes.

Comment 3

[Pardal Freudenthal (:ShareBird)]

Dão, using the DOMi to inspect the nodes on 3.6 and 4.0 I can see that on 3.6 the identity-box, textbox-input-box and urlbar-icons were inside a hbox (the autocomplete-textbox-container). I've used on 3.6 this box for my approach. For 4.0 I've used the same approach but applying to the new containers (I'm using a chrome.manifest flag to fire FF 4.0 urlbar stuff).

I've investigated this issue and I have no possibilities to maintain my urlbar appearance in the way it is now. That's why I'm filing this bug...

Notice that the addition of these containers will not affect any of our default themes. I also don't think that any third-party theme will work on FF 4.0 without many modifications, so I don't really see any annoyance for third-party themes at all.

Comment 4

[Dão Gottwald [:dao]]

(In reply to comment #3)
> Dão, using the DOMi to inspect the nodes on 3.6 and 4.0 I can see that on 3.6
> the identity-box, textbox-input-box and urlbar-icons were inside a hbox (the
> autocomplete-textbox-container). I've used on 3.6 this box for my approach. For
> 4.0 I've used the same approach but applying to the new containers (I'm using a
> chrome.manifest flag to fire FF 4.0 urlbar stuff).
>
> I've investigated this issue and I have no possibilities to maintain my urlbar
> appearance in the way it is now. That's why I'm filing this bug...

The inconsistency you see between Firefox 3.6 and trunk sounds exactly like what I mentioned in comment 2. Let's fix that.

> Notice that the addition of these containers will not affect any of our default
> themes.

Yes, they are less tied to the binding structure because they use the new urlbar-* classes. That's not the case for themes that are compatible with 3.6.

> I also don't think that any third-party theme will work on FF 4.0
> without many modifications, so I don't really see any annoyance for third-party
> themes at all.

Well, that there's a large pile of changes doesn't mean that adding more to the pile isn't a problem.

Comment 5

[Dão Gottwald [:dao]]

Attachment: patch

The theme adjustments let the vertical bar after the "Switch to tab:" label continue to touch the location bar border.

Comment 6

[Pardal Freudenthal (:ShareBird)]

(In reply to comment #5)
> Created attachment 510106 [details] [diff] [review]
> patch
> 
> The theme adjustments let the vertical bar after the "Switch to tab:" label
> continue to touch the location bar border.

Looking good. 
Sorry for my wrong approach...

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/5c77943a504d

Comment 8

[Teodosia Pop]

Verified using Mozilla/5.0 (Windows NT 6.1; rv:2.0b12) Gecko/20100101 Firefox/4.0b12.