Closed Bug 544657 Opened 12 years ago Closed 12 years ago

move urlbar emptytext code to the urlbar binding

Categories

(Firefox :: Address Bar, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.7a2

People

(Reporter: dao, Assigned: dao)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Just like the binding manages the other browser.urlbar.* prefs. No reason to clutter browser.js with this.
Attachment #425639 - Flags: review?(gavin.sharp)
Attachment #425639 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/mozilla-central/rev/b68fe179eaec
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a2
The Txul regression this patch presumably caused was never really fixed, was it?

http://graphs.mozilla.org/graph.html#tests=[{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22398%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22391%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22392%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22393%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22394%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22395%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22396%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22397%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22399%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22400%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22401%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22402%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22403%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22404%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22405%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22406%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22407%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22408%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22409%22},{%22test%22:%2217%22,%22branch%22:%221%22,%22machine%22:%22410%22}]
I don't know. I couldn't figure it out on the dashboard and graph.html doesn't give me graphs.

The emptytext is being set after a timeout in the xbl constructor, while it used to be set after a timeout from a load event listener. Since the xbl constructor precedes the load event, I guess I could try to increase the timeout, although that will cause problems with browser_bug495058.js.
(In reply to comment #3)
> I don't know. I couldn't figure it out on the dashboard and graph.html doesn't
> give me graphs.

The first thing graph.html does is load a 7mb file with a list of 42000 test machines... maybe you just need to be more patient ;)

> The emptytext is being set after a timeout in the xbl constructor, while it
> used to be set after a timeout from a load event listener. Since the xbl
> constructor precedes the load event, I guess I could try to increase the
> timeout, although that will cause problems with browser_bug495058.js.

Ah, so it's just moving the goalposts, and the regression is an artifact of the arbitrary point at which onload fires?
Well, setting the emptytext earlier could actually result in more work.
It looks like bug 547224 solved the perf issue.
Yay!
You need to log in before you can comment on or make changes to this bug.