remove unneeded/unused parts from the urlbar binding

VERIFIED FIXED in Firefox 3 beta1

Status

()

Firefox
Address Bar
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

({perf})

Trunk
Firefox 3 beta1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Created attachment 285856 [details] [diff] [review]
patch
Flags: blocking-firefox3?
Attachment #285856 - Flags: review?(gavin.sharp)
(Assignee)

Updated

11 years ago
Target Milestone: Firefox 3 M9 → ---
(Assignee)

Updated

11 years ago
Blocks: 396084
(Assignee)

Updated

11 years ago
Blocks: 396136
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?
Attachment #285856 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 2

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

Updated

11 years ago
Attachment #285856 - Flags: approvalM9?
Attachment #285856 - Flags: approval1.9?
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?
(Assignee)

Comment 4

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

Comment 6

11 years ago
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.
Flags: blocking-firefox3? → blocking-firefox3+
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.
Attachment #285856 - Flags: approvalM9?
Attachment #285856 - Flags: approvalM9+
Attachment #285856 - Flags: approval1.9?
Attachment #285856 - Flags: approval1.9+
(Assignee)

Updated

11 years ago
Target Milestone: --- → Firefox 3 M9
Keywords: checkin-needed
[03:25]	<Mano> gavin: btw, why is xmlns:html still there? :p

Updated

11 years ago
Whiteboard: [has patch][has reviews]
Created attachment 286240 [details] [diff] [review]
unbitrotten patch with nit addressed
Attachment #285856 - Attachment is obsolete: true
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
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Blocks: 386727
(Assignee)

Updated

11 years ago
Blocks: 394083
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
Status: RESOLVED → VERIFIED
(Assignee)

Updated

9 years ago
Whiteboard: [has patch][has reviews]
You need to log in before you can comment on or make changes to this bug.