707814 - Awesomebar autocomplete box appears in the wrong place

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[Johnathan Nightingale [:johnath]]

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. :\

Comment 1

[Johnathan Nightingale [:johnath]]

Could be bug 704521, cc'ng paul and mounir

(window: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c2102c45c8da&tochange=1bd7482ad4d1 )

Comment 2

[Johnathan Nightingale [:johnath]]

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.

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

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.

Comment 4

[:Margaret Leibovic]

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.

Comment 5

[:Margaret Leibovic]

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.

Comment 6

[Johnathan Nightingale [:johnath]]

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.

Comment 8

[Neil Deakin]

Some builds with that bug backed out are at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-146d06a96feb/

Comment 9

[Neil Deakin]

Margaret confirms that the build above works suggesting a regression from 668437.

I can't reproduce but others see it often.

Comment 10

[Timothy Nikkel (:tnikkel)]

Do people on Linux or Windows see this? Or is it mac-only?

Comment 11

[:Margaret Leibovic]

Another note: I saw this happen with form autocomplete as well.

Comment 12

[Timothy Nikkel (:tnikkel)]

And does this only show up if you use more than one screen?

Comment 13

[:Margaret Leibovic]

(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).

Comment 14

[Joe Drew (not getting mail)]

And, in fact, dragging the window manually *at all* seems to fix the problem, regardless of whether it's moved between screens.

Comment 15

[Timothy Nikkel (:tnikkel)]

So far I haven't been able to reproduce with the steps in comment 5 on a mac :(.

Comment 16

[Andrew McCreight [:mccr8] (PTO until 12-2)]

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.

Comment 17

[Timothy Nikkel (:tnikkel)]

That didn't work for me either. :(

Comment 18

[Andrew McCreight [:mccr8] (PTO until 12-2)]

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.

Comment 19

[Timothy Nikkel (:tnikkel)]

(In reply to Andrew McCreight [:mccr8] from comment #18)
> I'm on 10.6.8.

Same. I'll keep trying things.

Comment 21

[Timothy Nikkel (:tnikkel)]

I can reproduce now.

Comment 22

[Rob Campbell [:rc] (:robcee)]

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.

Comment 23

[Timothy Nikkel (:tnikkel)]

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.

Comment 24

[Markus Stange [:mstange]]

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?

Comment 25

[Timothy Nikkel (:tnikkel)]

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.

Comment 26

[Timothy Nikkel (:tnikkel)]

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

Comment 27

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

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 28

[Timothy Nikkel (:tnikkel)]

Comment on attachment 583930 [details] [diff] [review]
Patch v0.1

Seems reasonable to me, let's see what Markus thinks.

Comment 29

[Markus Stange [:mstange]]

Comment on attachment 583930 [details] [diff] [review]
Patch v0.1

If this fixes it, great!

Comment 30

[Timothy Nikkel (:tnikkel)]

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.

Comment 31

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(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

Comment 32

[Timothy Nikkel (:tnikkel)]

(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. :)

Comment 33

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/c4033a3d158e

Comment 34

[Alex Keybl [:akeybl]]

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.

Comment 35

[Timothy Nikkel (:tnikkel)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0908a812913f

Comment 36

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(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 :)

Comment 37

[juan becerra [:juanb]]

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.