1118135 - Clicking search field's magnifying glass icon no longer dismisses search field's dropdown menu

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Chris Peterson [:cpeterson]]

STR:
1. Click the search field's magnifying glass icon.
2. The search field's dropdown menu will open.
3. Click the search field's magnifying glass icon a second time.

RESULT:
The dropdown menu will reopen, but it should close (like it used). This bug is probably a regression from the search field's new "magnifying glass with plus symbol" icon.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Sounds potentially related to/caused by bug 1103455.

Comment 2

[Dave Townsend [:mossop]]

Almost certainly yes, should be straightforward to fix though.

Comment 3

[Dave Townsend [:mossop]]

Attachment: MozReview Request: bz://1118135/Mossop

Comment 4

[Dave Townsend [:mossop]]

/r/2321 - Bug 1118135: Clicking the magnifying glass while the suggestions are open should close the popup and not re-open it.

Pull down this commit:

hg pull review -r 521d024b33011d0f501a5cd6a1317363f585ffed

Comment 5

[Dave Townsend [:mossop]]

One thing I didn't mention in the commit message yet, I moved one of the tests to the end of browser_searchbar_openpopup.js as using synthesizeNativeMouseClick there was somehow causing the following test to timeout on Windows XP debug only. Never figured out why but putting at the end seems to solve the problem for now.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

https://reviewboard.mozilla.org/r/2319/#review1517

::: browser/base/content/urlbarBindings.xml
(Diff revision 1)
> +          this._isHiding = false;

Would it work if you used the `popuphidden` event to set `_isHiding` back to false instead of using the setTimeout?

Comment 7

[Dave Townsend [:mossop]]

(In reply to :Felipe Gomes from comment #6)
> https://reviewboard.mozilla.org/r/2319/#review1517
> 
> ::: browser/base/content/urlbarBindings.xml
> (Diff revision 1)
> > +          this._isHiding = false;
> 
> Would it work if you used the `popuphidden` event to set `_isHiding` back to
> false instead of using the setTimeout?

No, popuphidden fires before any of the mouse events make it to the DOM

Comment 8

[:Felipe Gomes (needinfo for replies!)]

https://reviewboard.mozilla.org/r/2319/#review1547

Ship It!

Comment 9

[Dave Townsend [:mossop]]

https://hg.mozilla.org/integration/fx-team/rev/036dce7553a1

Comment 10

[Dave Townsend [:mossop]]

[Tracking Requested - why for this release]: Regression caused by bug 1103455

Not sure who marked 36 as unaffected, both 35 and 36 are affected but we're too late for 35 now.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/036dce7553a1

Comment 12

[Ashwini K H]

Bug Verification Day-2015-14-01

Comment 13

[Dave Townsend [:mossop]]

Comment on attachment 8546884 [details]
MozReview Request: bz://1118135/Mossop

Approval Request Comment
[Feature/regressing bug #]: bug 1103455
[User impact if declined]: Clicking the magnifying glass just closes then re-opens the search popup
[Describe test coverage new/current, TBPL]: Automated tests running on m-c since landing
[Risks and why]: Low risk, the only in-product piece of this is to ignore click events when certain other events have happened recently. Worst case in some instances clicking the magnifying glass might not open the popup at all but we have automated tests covering the common cases of that.
[String/UUID change made/needed]: None

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/baf867561ca9
https://hg.mozilla.org/releases/mozilla-beta/rev/ee9df2674663

Comment 16

[Dave Townsend [:mossop]]

Attachment: MozReview Request: Bug 1118135: Clicking the magnifying glass while the suggestions are open should close the popup and not re-open it.