Closed
Bug 707814
Opened 13 years ago
Closed 13 years ago
Awesomebar autocomplete box appears in the wrong place
Categories
(Firefox :: Address Bar, defect)
Tracking
()
RESOLVED
FIXED
Firefox 12
People
(Reporter: johnath, Assigned: zpao)
References
Details
(Whiteboard: [qa!])
Attachments
(1 file)
(deleted),
patch
|
mstange
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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. :\
Reporter | ||
Comment 1•13 years ago
|
||
Could be bug 704521, cc'ng paul and mounir
(window: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c2102c45c8da&tochange=1bd7482ad4d1 )
Reporter | ||
Comment 2•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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.
Reporter | ||
Comment 6•13 years ago
|
||
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•13 years ago
|
||
Some builds with that bug backed out are at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@mozilla.com-146d06a96feb/
Comment 9•13 years ago
|
||
Margaret confirms that the build above works suggesting a regression from 668437.
I can't reproduce but others see it often.
Blocks: 668437
Comment 10•13 years ago
|
||
Do people on Linux or Windows see this? Or is it mac-only?
Comment 11•13 years ago
|
||
Another note: I saw this happen with form autocomplete as well.
Comment 12•13 years ago
|
||
And does this only show up if you use more than one screen?
Updated•13 years ago
|
tracking-firefox11:
--- → ?
Comment 13•13 years ago
|
||
(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•13 years ago
|
||
And, in fact, dragging the window manually *at all* seems to fix the problem, regardless of whether it's moved between screens.
Comment 15•13 years ago
|
||
So far I haven't been able to reproduce with the steps in comment 5 on a mac :(.
Comment 16•13 years ago
|
||
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•13 years ago
|
||
That didn't work for me either. :(
Comment 18•13 years ago
|
||
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•13 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #18)
> I'm on 10.6.8.
Same. I'll keep trying things.
Comment 21•13 years ago
|
||
I can reproduce now.
Comment 22•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
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)?
Assignee | ||
Comment 27•13 years ago
|
||
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•13 years ago
|
||
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 29•13 years ago
|
||
Comment on attachment 583930 [details] [diff] [review]
Patch v0.1
If this fixes it, great!
Attachment #583930 -
Flags: review?(mstange) → review+
Comment 30•13 years ago
|
||
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.
Assignee | ||
Comment 31•13 years ago
|
||
(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]
Comment 32•13 years ago
|
||
(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•13 years ago
|
||
Assignee: nobody → paul
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 12
Updated•13 years ago
|
Attachment #583930 -
Flags: approval-mozilla-aurora?
Updated•13 years ago
|
Comment 34•13 years ago
|
||
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+
Updated•13 years ago
|
status-firefox11:
--- → fixed
Comment 35•13 years ago
|
||
Assignee | ||
Comment 36•13 years ago
|
||
(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•13 years ago
|
||
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.
Description
•