Autocomplete suggestions just don't go away after Win+Down

RESOLVED FIXED in Firefox 48

Status

()

defect
P2
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: arni2033, Assigned: enndeakin)

Tracking

(Depends on 1 bug, Blocks 1 bug, {regression})

48 Branch
Firefox 49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 unaffected, firefox48+ fixed, firefox49+ fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160426044609
STR:
1. Open new window, switch it to normal (not maximized) mode
2. Focus urlbar, type "a"
3. Press Win+Down
4. Click button on Windows Taskbar to open the window again
5.(bonus) Focus searchbar, type "b". Move the window a bit (drag-n-drop)

AR:  After step 4 autocomplete suggestions are open. They don't hide/move in Step 5.
ER:  Either no autocomplete suggestions after step 4, or something more smart.

This is regression from bug 1264983. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=9774863180f92d75e7851f138bed71fe9828122a&tochange=45f98cbddc015c5659abe141de0b8fa6bd534051
(Reporter)

Updated

3 years ago
See Also: → 1192501
annoyance, but not trivial to hit.
If it's related to a platform bug with toplevel popups it could also be expensive to fix.
Priority: -- → P4
Whiteboard: [fxsearch]
(Reporter)

Comment 2

3 years ago
(In reply to Marco Bonardo [::mak] from comment #1)
> annoyance, but not trivial to hit.
Simply press Win+Down. How is that not trivial?

> If it's related to a platform bug with toplevel popups it could also be expensive to fix.
Back out bug 1264983 then?
An "improvement" isn't a good excuse to have several way worse bugs, unless that's your goal.
Flags: needinfo?(mak77)
(In reply to arni2033 from comment #2)
> (In reply to Marco Bonardo [::mak] from comment #1)
> > annoyance, but not trivial to hit.
> Simply press Win+Down. How is that not trivial?

Ah wait, step 5 confused my tries to reproduce the bug. sorry for the confusion, yes, this is worth fixing. I don't think we should backout bug 1264983, since that would introduce a bug in OS X to workaround one on Windows. We should rather try to fix this if possible and keep the backout as a last opportunity.

The first 4 steps make so that the popup is "stuck" open forever, until you type again in the location bar (and then we reposition it).

Maybe Neil knows if this is a known issue with level="parent" or it's something we are not handling
Flags: needinfo?(mak77) → needinfo?(enndeakin)
Priority: P4 → P2
(Reporter)

Comment 4

3 years ago
(In reply to Marco Bonardo [::mak] from comment #3)
Luckily you read my replies...

> I don't think we should backout bug 1264983, since that would introduce a bug in OS X
> to workaround one on Windows.
> If it's related to a platform bug with toplevel popups
If this is Windows-specific (as said in bug 1268735 comment 1), you could (probably) just back out it 
from Windows and have enough time to get rid of _this_ bug. That's all my suggestions as of now.
(In reply to arni2033 from comment #4)
> (In reply to Marco Bonardo [::mak] from comment #3)
> Luckily you read my replies...

It's not luck, I read hundreds bugzilla comments every day!

> If this is Windows-specific (as said in bug 1268735 comment 1), you could
> (probably) just back out it 
> from Windows and have enough time to get rid of _this_ bug. That's all my
> suggestions as of now.

that's a possibility we can consider, a windows only ifndef. But first let's see what Neil thinks of the issue.
(Assignee)

Comment 6

3 years ago
The popup is being closed during the minimize at step 4 but it looks like Windows for some reason decides to show the popup again itself when the main window is unminimized. However, we don't think the popup is open so no events get routed to it.

I'll investigate why this might be.
Flags: needinfo?(enndeakin)
(Assignee)

Comment 7

3 years ago
Posted patch Hide popups on minimize (obsolete) — Splinter Review
This patch seems to work, but is a bit aggressive in that it closes all popups whenever any window is minimized.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
(Assignee)

Comment 8

3 years ago
Comment on attachment 8757391 [details] [diff] [review]
Hide popups on minimize

OK, so background windows don't seem to get WM_SHOWWINDOW so this is probably ok, and WM_WINDOWPOSCHANGED doesn't give enough information in this case.
Attachment #8757391 - Flags: review?(jmathies)

Updated

3 years ago
Attachment #8757391 - Flags: review?(jmathies) → review+
The win+down arrow trick doesn't impact select dropdown windows. I wonder what the difference is here?
(Assignee)

Comment 10

3 years ago
They are toplevel popups whereas the autocomplete popup was changed in bug 1264983 to not be toplevel. This means that it does not have WS_EX_TOPMOST set.
(Assignee)

Comment 11

3 years ago
Forgot the return at the end.
Attachment #8757391 - Attachment is obsolete: true
(Assignee)

Comment 12

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fefb7adc384abcc33f4da47496787bfdac1c9f85
Bug 1276120, hide popups when a parent window is being minimized, handles the case where the window is minimized via a keyboard shortcut, r=jmathies
[Tracking Requested - why for this release]: It's a new regression for the awesomebar redesign

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fefb7adc384a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Tracking since this is a recent regression. Marco, is this something you intend to uplift to beta?
Flags: needinfo?(enndeakin)
Flags: needinfo?(mak77)
ehr sorry, yes it would be great to have this in beta.
But I can't comment regarding the risk, I prefer Neil or Jim to evaluate that.
Flags: needinfo?(mak77)
(Assignee)

Comment 17

3 years ago
I guess it has less risk than 1264983.
Flags: needinfo?(enndeakin)
OK, could you fill the uplift request then?
Thanks
Flags: needinfo?(mak77)
Flags: needinfo?(enndeakin)
Comment on attachment 8759749 [details] [diff] [review]
Hide popups on minimize, v2

Approval Request Comment
[Feature/regressing bug #]: new awesomebar design
[User impact if declined]: The awesomebar popup detaches and covers the browser window if it was minimized using keyboard shortcuts.
[Describe test coverage new/current, TreeHerder]: nightly/aurora
[Risks and why]: less risky than the fix that caused this regression (bug 1264983), should be an easy backout in case issues are found (just a few lines of code)
[String/UUID change made/needed]: none
Flags: needinfo?(mak77)
Flags: needinfo?(enndeakin)
Attachment #8759749 - Flags: approval-mozilla-beta?
Comment on attachment 8759749 [details] [diff] [review]
Hide popups on minimize, v2

Review of attachment 8759749 [details] [diff] [review]:
-----------------------------------------------------------------

The patch fixes a regression. Take it in 48 beta 5.
Attachment #8759749 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [good first verify]
Version: unspecified → 48 Branch
Reproduced the bug in firefox nightly 49.0a1 (2016-05-26) with windows 10 (64 bit)

Verified as fixed with latest firefox beta 48.0b7 (Build ID: 20160711002726)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0

and latest firefox aurora 49.0a2 (Build ID: 20160711004013)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
QA Whiteboard: [good first verify] → [good first verify] [bugday-20160713]

Comment 23

3 years ago
Verified the bug as fixed on Firefox Aurora 49.0a2.

[testday-20160722]

Comment 24

3 years ago
I reproduced the issue using Firefox nightly 49.0a1 (2016-05-26) on Windows 10 Pro x86_64.
I have verified the fix on the latest Beta 49.0b7.

Build ID: 20160825132718
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0

[testday-20160826]
You need to log in before you can comment on or make changes to this bug.