move urlbar emptytext code to the urlbar binding

RESOLVED FIXED in Firefox 3.7a2

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 3.7a2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Created attachment 425639 [details] [diff] [review]
patch

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+
(Assignee)

Comment 1

9 years ago
http://hg.mozilla.org/mozilla-central/rev/b68fe179eaec
Status: ASSIGNED → RESOLVED
Last Resolved: 9 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}]
(Assignee)

Comment 3

9 years ago
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?
(Assignee)

Comment 5

9 years ago
Well, setting the emptytext earlier could actually result in more work.
(Assignee)

Comment 6

9 years ago
It looks like bug 547224 solved the perf issue.
You need to log in before you can comment on or make changes to this bug.