Comment on attachment 285856 [details] [diff] [review] patch >Index: toolkit/content/widgets/autocomplete.xml > <binding id="autocomplete" > extends="chrome://global/content/bindings/textbox.xml#textbox"> > <content sizetopopup="pref"> > <xul:hbox class="autocomplete-textbox-container" flex="1"> >- <children includes="image|deck|stack"> >+ <children includes="image|deck|stack|box"> What's this change about? Unintentional?

(In reply to comment #1) > (From update of attachment 285856 [details] [diff] [review]) > >Index: toolkit/content/widgets/autocomplete.xml > > > <binding id="autocomplete" > > extends="chrome://global/content/bindings/textbox.xml#textbox"> > > > <content sizetopopup="pref"> > > <xul:hbox class="autocomplete-textbox-container" flex="1"> > >- <children includes="image|deck|stack"> > >+ <children includes="image|deck|stack|box"> > > What's this change about? Unintentional? That's for the identity box from bug 383183.

Oh, right... That's suboptimal, I was rather wanting to avoid having to change a toolkit binding for the identity handler specifically, but I guess there isn't much of an alternative. Does duplicating the autocomplete binding's <content> and making that single change cause a perf hit?

I wouldn't expect any perf hit, the simple fact that we would be duplicating the entire thing just to change one value seemed suboptimal to me. I don't really like adding "box" there either, but then most of the remaining "includes" rules aren't very intuitive.

(In reply to comment #4) > I wouldn't expect any perf hit, the simple fact that we would be duplicating > the entire thing just to change one value seemed suboptimal to me. Yeah, but the alternative is a change to a global binding that might affect other people that use it and put <box>es in it. Maybe I'm worrying too much.

To this inexperienced user, from looking at the patch, it looks like it removes the browser.urlbar.hideProtocols and browser.urlbar.animateBlend prefs. I currently have hideProtocols empty and animateBlend set to false because I don't find much benefit from hiding the protocol and the location bar flickering was getting annoying (bug 396084). I suspect I am not entirely alone in being happy (especially among more advanced Firefox users) with the way the location bar behaves without the fading and the hiding of the http and https bits, although many users I'm sure will find that useful. Is it possible to retain a way for those who don't find these features useful to disable them?

It removes the prefs because it's removing the feature.

Comment on attachment 285856 [details] [diff] [review] patch a=endgame drivers for M9 checkin. Due to the likely perf impact of this patch, please land this on a quiet tree.

[03:25] <Mano> gavin: btw, why is xmlns:html still there? :p

Checking in browser/app/profile/firefox.js; /cvsroot/mozilla/browser/app/profile/firefox.js,v <-- firefox.js new revision: 1.214; previous revision: 1.213 done Checking in browser/base/content/urlbarBindings.xml; /cvsroot/mozilla/browser/base/content/urlbarBindings.xml,v <-- urlbarBindings.xml new revision: 1.38; previous revision: 1.37 done Checking in browser/themes/pinstripe/browser/browser.css; /cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v <-- browser.css new revision: 1.89; previous revision: 1.88 done Checking in browser/themes/winstripe/browser/browser.css; /cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v <-- browser.css new revision: 1.109; previous revision: 1.108 done Checking in toolkit/content/widgets/autocomplete.xml; /cvsroot/mozilla/toolkit/content/widgets/autocomplete.xml,v <-- autocomplete.xml new revision: 1.85; previous revision: 1.84 done

Verified via a combination of both Bonsai and UI/functionality inspection; we no longer do neither the protocol hiding nor the fade/hover effects on the following platforms: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b4pre) Gecko/2008020604 Minefield/3.0b4pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008020604 Minefield/3.0b4pre Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008020604 Minefield/3.0b4pre