544657 - move urlbar emptytext code to the urlbar binding

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

Just like the binding manages the other browser.urlbar.* prefs. No reason to clutter browser.js with this.

Comment 1

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/b68fe179eaec

Comment 2

[Markus Stange [:mstange]]

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}]

Comment 3

[Dão Gottwald [:dao]]

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.

Comment 4

[Markus Stange [:mstange]]

(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?

Comment 5

[Dão Gottwald [:dao]]

Well, setting the emptytext earlier could actually result in more work.

Comment 6

[Dão Gottwald [:dao]]

It looks like bug 547224 solved the perf issue.

Comment 7

[Markus Stange [:mstange]]

Yay!