Closed Bug 1489780 Opened 6 years ago Closed 6 years ago

'Firefox Home' page search option breaks when it's moved to a new window

Categories

(Toolkit :: Async Tooling, defect, P1)

64 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla67
Fission Milestone M1
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: johnsonmcbig, Assigned: Felipe)

References

Details

(Keywords: regression)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0 Build ID: 20180908100402 Steps to reproduce: 1. Open Firefox Nightly 2. Grab a tab 3. drop it in a random position to create a new window or into an existing Firefox window 4. Click on the in-page search bar 5. Enter any search query(or none) 6. Attempt to send the query by pressing the arrow on the right side of the search bar or by pressing the 'Enter' key on the keyboard Actual results: Once you try to send the query, it just won't work, nothing happens. Expected results: It should have functioned just like it does when you get the page in a fresh Firefox instance or when you create a new tab, it should've ran the entered search query through the specified search engine.
I must stress that I am talking about the 'Firefox Home' page
Component: Untriaged → Search
Browser console shows: TypeError: this.listeners is null, can't access property "push" of it ActorManagerChild.jsm:176:5 Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d5c898a4aaafedae7c53131adb5ca910c054df85&tochange=9b5aefe66457f3bbb6403dbe7c149993c7c7eb70 Suspect Bug 1483738
Blocks: 1483738
Status: UNCONFIRMED → NEW
Ever confirmed: true
Or maybe Bug 1483664
Blocks: 1483664
No longer blocks: 1483738
Priority: -- → P1
The error in comment 2 happens when the tab is moved, not when you try to search. The stack is: > addEventListener@resource://gre/modules/ActorManagerChild.jsm:190:66 > init@resource://gre/modules/ActorManagerChild.jsm:50:7 > init@resource://gre/modules/ActorManagerChild.jsm:143:5 > handleEvent@resource://gre/modules/ActorManagerChild.jsm:180:9 > EventListener.handleEvent*SingletonDispatcher@resource://gre/modules/ActorManagerChild.jsm:135:5 > onNewDocument@resource://gre/modules/ActorManagerChild.jsm:228:5 > MozDocumentCallback.onNewDocument*init@resource://gre/modules/ActorManagerChild.jsm:217:21 > @resource://gre/modules/ActorManagerChild.jsm:258:1 > @chrome://global/content/browser-content.js:12:1 Kris, could you please look at this? You added the related tooling in bug 1472491 it looks like. See also the bugs and regression range that Alice points to above.
Component: Search → Async Tooling
Flags: needinfo?(kmaglione+bmo)
Product: Firefox → Toolkit
Yoric, can you help find an owner for this issue? Seems like it should be more of a priority; it affects search functionality and we have a regression range to work with. Thanks!

I don't think that felipe is working on these things these days, but he probably knows better than me who would.

Flags: needinfo?(dteller) → needinfo?(felipc)

Yeah, this is on me (very similar to bug 1499896). I don't think this is critical, since it requires opening a new tab and detaching it to a new window, but I'm planning on doing some work in this area this week and I'll make sure to fix this too.

Assignee: nobody → felipc
Status: NEW → ASSIGNED
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(felipc)

The user-visible bug reported here is not happening anymore on the current Nightly because there were UI changes, but the underlying issue is still around and could affect other actors.

The problem is that the _dispatcher is assigned to the actor on the constructor and not in init(), but it was cleaned up on cleanup() (which is meant to be used as an init/cleanup pair), so it didn't get assigned again on the pageshow after the tab gets to the new window.

There really is no need to do this manual cleanup for the _dispatcher, so just removing it is the easiest.

This bug is marked as p1 and the patch looks minimal, Felipe, do you think that once landed you patch would be a potential candidate for uplifting to Beta? Thanks

Flags: needinfo?(felipc)

(In reply to Pascal Chevrel:pascalc from comment #10)

This bug is marked as p1 and the patch looks minimal, Felipe, do you think
that once landed you patch would be a potential candidate for uplifting to
Beta? Thanks

Yep. I'll nominate it when it lands

Flags: needinfo?(felipc)
Fission Milestone: --- → M1
Whiteboard: [2/14] patch waiting for kmag's review
Whiteboard: [2/14] patch waiting for kmag's review → [2/21] patch r+, ready to land
Pushed by fgomes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/106c2c83d883 Avoid unnecessary clean-up on ActorChild. r=mconley
Whiteboard: [2/21] patch r+, ready to land → [2/21] patch pushed
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Is this something which should be considered for Beta backport or can it ride the trains?

Flags: qe-verify+
Flags: needinfo?(felipc)
Depends on: 1530019

(In reply to Ryan VanderMeulen [:RyanVM] from comment #14)

Is this something which should be considered for Beta backport or can it
ride the trains?

If we're not seeing duplicates of this problem filed, or reports somewhere else on the web, I'd rather let this ride the trains..

Flags: needinfo?(felipc)
Whiteboard: [2/21] patch pushed

Reproduced the issue on affected Nightly 64.0a1 (2018-09-08) (64-bit)

Now it is verified - fixed on the latest Firefox Beta 67.0b3 (64-bit) on Windows 10, Mac OS 10.13 and Ubuntu 16.04.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: