Closed Bug 707814 Opened 13 years ago Closed 12 years ago

Awesomebar autocomplete box appears in the wrong place

Categories

(Firefox :: Address Bar, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 12
Tracking Status
firefox11 + verified

People

(Reporter: johnath, Assigned: zpao)

References

Details

(Whiteboard: [qa!])

Attachments

(1 file)

After applying an update and restarting, typing in the awesomebar caused the autocomplete box to appear in the wrong position. In particular, it was at y=0, top of the window, instead of appearing beneath the location bar. Sadly, I didn't screen cap and my memory is fuzzy, but I don't believe it was at x=0.

Joe saw this when he updated to today's nightly, but couldn't reproduce it. A few hours later, I updated to today's nightly and saw the same thing, but now can't reproduce it either. :\
I know he hates it when people add him to bugs just because XUL is implicated, but on the off chance that it has anything to do with Mounir's patch, it involves focus too, so.... adding Enn.
I can't seem to repro at all :/

Bug 704521 shouldn't be at fault, though I can understand it being blamed since it touched autocomplete (sort of). I don't think the location bar uses form fill controller though, just the search box.
This has happened to me multiple times today, and it seems to be triggered by plugging in/unplugging my external monitor. I can try to get reliable steps to reproduce tomorrow if it happens again.
Yes, STR (latest Nightly):

1) Plug in external monitor and move Firefox window to that monitor
2) Unplug external monitor (the window should move back to your laptop screen)
3) Try typing in the awesomebar, the popup will be in the wrong place

This also happens again when you plug your monitor back in and the window moves back to the monitor.

My external monitor is above my laptop screen, and my Firefox window is at the top of the screen in laptop-maximized size. When the popup is on the wrong place on the external monitor is is too low, and when it's in the wrong place on my laptop screen it's too high, so it looks like screen coordinates are getting saved but not updated when the monitor switch happens.
Yep - margaret's STR wfm. Neil is wondering (out loud) if maybe it is bug 668437 and is gonna throw together a backout build to try unless someone beats him to it.
Margaret confirms that the build above works suggesting a regression from 668437.

I can't reproduce but others see it often.
Blocks: 668437
Do people on Linux or Windows see this? Or is it mac-only?
Another note: I saw this happen with form autocomplete as well.
And does this only show up if you use more than one screen?
(In reply to Timothy Nikkel (:tn) from comment #12)
> And does this only show up if you use more than one screen?

Yeah, I only see this when my window moves between screens, and only when it moves itself because one screen has been disconnected (it works fine if I drag the window myself).
And, in fact, dragging the window manually *at all* seems to fix the problem, regardless of whether it's moved between screens.
So far I haven't been able to reproduce with the steps in comment 5 on a mac :(.
I see this even when the window doesn't move between screens.  I have my external monitor set to be the primary window, my Firefox window is on my laptop monitor.  The Firefox window is the current active window.  Unplug the external monitor, causing the laptop monitor to become the active window.  Typing in the awesomebar causes the autocomplete to appear in the wrong place.
That didn't work for me either. :(
Okay.  The window was currently active, and my only interaction with it was to hit command-L before typing.  Also having the awesomebar active (with blinking cursor and all) before unplugging, then just typing, works.  I was also able to type out this message after monitor unplugging, then hit command-L to go to the awesomebar and start typing, and it was messed up.  I'm on 10.6.8.
(In reply to Andrew McCreight [:mccr8] from comment #18)
> I'm on 10.6.8.

Same. I'll keep trying things.
I can reproduce now.
I was able to reproduce this today in a development build. I started the browser on one "Space" in OS X and moved the window into another Space while the browser was still starting up. Now I'm seeing my awesomebar's completion dropdown in a funny place.

also, can confirm that moving the window a pixel fixes the positioning.
In nsMenuPopupFrame::SetPopupPosition when we call GetClientOffset on the widget for the awesomebar drop down after it was shown on a screen that no longer exists but before we've shown it on the new screen we get an invalid result because it subtracts mBounds from the result of GetClientRect, where mBounds is out of date compared to GetClientRect because GetClientRect makes a system call to get the client rect. Calling UpdateBounds in GetClientOffset fixes the bug. So it seems to be a case of making sure mBounds gets updated in the proper place.
Maybe GetClientBounds should use nsCocoaUtils::GeckoRectToCocoaRect(mBounds) instead of [mWindow frame], so that we don't have the discrepancy between out-of-date and up-to-date values.
But the fact that mBounds is out of date is troubling. Is windowDidMove not called in this scenario? Or is it called too late?
At what point do the [mWindow frame] values change between the ones we set (and copy to mBounds) and the ones that GetClientBounds gets? Does it happen during nsCocoaWindow::Show? If so, maybe we should add a call to UpdateBounds in nsCocoaWindow::Show.
Do we fire NS_MOVE events at all when the widget switches screens? Should we?
The widget is hidden when the screen switch occurs. We don't get any windowDid/WillMove/Resize at all when the screen is removed and it switches screens. The mBounds next gets updated when we open the awesome bar again and we explicitly call Resize on the widget.
So is there any event we can listen for when the screen switch happens to update the stored bounds? Or should we just use mBounds in GetClientBounds (which works for me)?
Attached patch Patch v0.1Splinter Review
I've been mucking around in Cocoa widget code so I took a quick stab at this... it seems to be working.

It turns out there's a NSWindowDidChangeScreenNotification we can jump onto. Initially I just made UpdateBounds public & called that directly, but I reverted that & call ReportMoveEvent (since that calls UpdateBounds). I'm naive here so I don't know if this is going to be the wrong thing to do (as mentioned in comment 24)

(also, this is on top of my full screen patch - bug 639705 - so it doesn't look like it's going to apply cleanly)
Comment on attachment 583930 [details] [diff] [review]
Patch v0.1

Seems reasonable to me, let's see what Markus thinks.
Attachment #583930 - Flags: review?(mstange)
Comment on attachment 583930 [details] [diff] [review]
Patch v0.1

If this fixes it, great!
Attachment #583930 - Flags: review?(mstange) → review+
Paul, are you planning on landing this shortly? Or would you like me too? I'd like this to land this soon so we can get it into Aurora.
(In reply to Timothy Nikkel (:tn) from comment #30)
> Paul, are you planning on landing this shortly? Or would you like me too?
> I'd like this to land this soon so we can get it into Aurora.

Yea sorry, holiday here in the US.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4033a3d158e
Whiteboard: [inbound]
(In reply to Paul O'Shannessy [:zpao] from comment #31)
> Yea sorry, holiday here in the US.

It's a holiday here too. I was volunteering to land it for you assuming you were taking holidays. :)
https://hg.mozilla.org/mozilla-central/rev/c4033a3d158e
Assignee: nobody → paul
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 12
Attachment #583930 - Flags: approval-mozilla-aurora?
Comment on attachment 583930 [details] [diff] [review]
Patch v0.1

[Triage Comment]
This looks like agood, low-risk fix for multi-screen setups. Given where we are in the cycle, approving for Aurora.
Attachment #583930 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Timothy Nikkel (:tn) from comment #32)
> (In reply to Paul O'Shannessy [:zpao] from comment #31)
> > Yea sorry, holiday here in the US.
> 
> It's a holiday here too. I was volunteering to land it for you assuming you
> were taking holidays. :)

Bah of course it was a holiday there too (silly US holidays like President's day were on my mind). Thanks for the offer & then landing on Aurora :)
Whiteboard: [qa+]
Verified on Mac OS X 10.7.2 using Fx13(nightly) and Fx11b4(beta) using the steps in comment #5. Before fix I was able to see the problem, after the fix the autocomplete box is in the right place.
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.