Placeholder text disappears in SearchBar after customize

VERIFIED FIXED in Firefox 27

Status

()

defect
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: alice0775, Assigned: dao)

Tracking

({regression})

27 Branch
Firefox 29
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 unaffected, firefox27+ verified, firefox28+ verified, firefox29+ verified, firefox-esr24+ verified, b2g-v1.2 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

6 years ago
This problem is reproduced in Firefox27b6 and Aurora28.0a2, Holly29.0a1 and Nightly29.0a1.

Steps To Reproduce:
1. Start Firefox with newly created profile
2. Enter Customize toolbar mode
3. Exit Customize toolbar mode

Actual Results:
Placeholder text disappears in SearchBar
Reporter

Comment 1

6 years ago
Regression window(fx-team)
Good:
http://hg.mozilla.org/integration/fx-team/rev/18acca8fc681
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140109080042
Bad:
http://hg.mozilla.org/integration/fx-team/rev/1c7b312a2a85
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140109093219
http://hg.mozilla.org/integration/fx-team/rev/113e065a2803
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140109100819
Pushlog:
http://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=18acca8fc681&tochange=1c7b312a2a85

	f3e7fa7d9748	Gavin Sharp — Bug 903274: have the search bar binding's initialization callback bail out if the binding is destroyed, r=MattN
Blocks: 903274
Keywords: regression
Reporter

Comment 2

6 years ago
Firefox24.2pre also regressed due to Bug 903274 on Jan/11/2014.
http://hg.mozilla.org/releases/mozilla-esr24/rev/0a9219e71d17
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20140116 Firefox/24.0 ID:20140116000501
Reporter

Updated

6 years ago
Component: Toolbars and Customization → Search
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [triage] [defect] p=0
Assignee

Updated

6 years ago
Assignee: nobody → dao
Assignee

Comment 3

6 years ago
Posted patch patch (obsolete) — Splinter Review
This makes _destroyed the exact opposite of _addedObserver. I started unifying them, but ended up removing the latter, since bug 722332 made it so that the observer is added directly in the constructor rather than asynchronously.
Attachment #8361624 - Flags: review?(gavin.sharp)
We should just back out bug 903274 on early branches, and only take this followup on trunk.
(In reply to Dão Gottwald [:dao] from comment #3)
> since bug 722332 made it so that the observer is added directly in the constructor
> rather than asynchronously.

(https://hg.mozilla.org/mozilla-central/rev/9f3af99c8297 for the record)
Attachment #8361624 - Flags: review?(gavin.sharp) → review+
Flags: needinfo?(ryanvm)
Bug 903274 backed out from the release branches.
(In reply to Dão Gottwald [:dao] from comment #7)
> https://hg.mozilla.org/integration/fx-team/rev/4d2cefab5e0f

This broke all the mochitest-browser-chrome tests that open customize mode, on all platforms, because the constructor's addObserver doesn't get run in customize mode, and your change means the destructor unconditionally tries to call removeObserver, which throws NS_ERROR_FAILURE in this case. Maybe:

-        var os = Components.classes["@mozilla.org/observer-service;1"]
-                           .getService(Components.interfaces.nsIObserverService);
-        os.removeObserver(this, "browser-search-engine-modified");
+        if (this.parentNode.parentNode.localName != "toolbarpaletteitem")
+          var os = Components.classes["@mozilla.org/observer-service;1"]
+                             .getService(Components.interfaces.nsIObserverService);
+          os.removeObserver(this, "browser-search-engine-modified");
+        }


? Or would you prefer to back out?

(that seems to fix it locally - but locally I'm also seeing leaks, which aren't fixed by that change. I don't know why that is.)
Flags: needinfo?(dao)
Is there a better way in Australis for a node to know whether it's being customized rather than actually used? That's what that parentNode check was intended to do. At the very least we should use a more descriptive getter in the searchbar binding rather than calling getAttribute in both the destructor and constructor.

Comment 12

6 years ago
I don't know if it's related but the search bar is currently broken in customize mode without any text appearing in the palette and with the image half cut.
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #11)
> Is there a better way in Australis for a node to know whether it's being
> customized rather than actually used? That's what that parentNode check was
> intended to do. At the very least we should use a more descriptive getter in
> the searchbar binding rather than calling getAttribute in both the
> destructor and constructor.

document.documentElement.getAttribute("customizing") == "true"

is probably the easiest, assuming that's accessible in the destructor. If we're guaranteed to be in a browser window (is this binding used anywhere else?) then (window.)  CustomizationHandler.isCustomizing() will also give you what you want.
Assignee

Updated

6 years ago
Attachment #8363706 - Flags: review?(gavin.sharp)
Comment on attachment 8363706 [details] [diff] [review]
patch v2

thanks
Attachment #8363706 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/mozilla-central/rev/1061661a1119
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Duplicate of this bug: 963517
Verified as fixed on FF 27 beta 9 (build ID: 20140123185438).
Reporter

Updated

6 years ago
Duplicate of this bug: 963832

Comment 21

6 years ago
Verified as fixed with the steps from comment 0 (steps I previously reproduced the bug with) on the latest Firefox 28.0a2, Firefox 29.0a1 and Firefox 24.2.0esrpre (January 26th and 27th builds), on Windows 7 64bit, Mac OS X 10.7.5 and Ubuntu 13.04 32bit.
No longer blocks: fxdesktopbacklog
Whiteboard: [triage] [defect] p=0
You need to log in before you can comment on or make changes to this bug.