Elbart Reporter
	Description • 10 years ago

Bug 1110678 added the feature to change the default searchengine with keyboard-shortcut. Changing the engine doesn't close the searchpanel.

When using Ctrl+mousewheel to change the default searchengine, the searchpanel closes and reopens.

See the attached video for demonstration.

	Florian Quèze [:florian]
	Comment 1 • 10 years ago

Thanks for the bug report! I wonder if this could somehow have the same root cause as bug 1123292 where the panel also gets hidden when it shouldn't.

[Tracking Requested - why for this release]: edge case of bug 1110678 which was uplifted to 36.

	Elbart Reporter
	Comment 2 • 10 years ago

I don't know if it's intended, but Ctrl+Mousewheel is only registered/executed when the cursor is over the searchbar, not when it's over the searchpanel.

	Florian Quèze [:florian]
	Comment 3 • 10 years ago

(In reply to Elbart from comment #2)
> I don't know if it's intended, but Ctrl+Mousewheel is only
> registered/executed when the cursor is over the searchbar, not when it's
> over the searchpanel.

I think it's intended in that the code was written to work that way; that doesn't mean it's the best possible UX here.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 4 • 10 years ago

Although this bug affects the search UI, as stated in comment 1, this is an edge case. I don't think that we need to track this bug but I'm happy to consider an uplift request.

	Florian Quèze [:florian]
	Comment 5 • 10 years ago

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> I wonder if this could somehow have the same root
> cause as bug 1123292 where the panel also gets hidden when it shouldn't.

Now that bug 1123292 has been investigated a bit by Neil, I'm pretty sure the cause here is different.

	Florian Quèze [:florian]
	Comment 6 • 10 years ago

The bug also happens on Linux and Mac.

I looked into this a bit more, here's what happens: when the panel is closed, the searchbox receives the DOMMouseScroll event at http://hg.mozilla.org/mozilla-central/annotate/7c5f187b65bf/browser/components/search/content/search.xml#l600

When the panel is opened, the mouse event gets somehow eaten by the panel, and the panel gets closed (I don't know why exactly).

Neil, I was hoping this could have got fixed as a side effect of your patch in bug 1116865, but I just tested this on a current trunk build and I can still reproduce the bug. Do you happen to have a guess at what needs fixing here based on the investigation you've done for the previous bugs? This is not a high priority bug, but if the fix turns out to be trivial, it would be nice to uplift it.

	Neil Deakin
	Comment 7 • 10 years ago

The expected behaviour of popups is to close them when the mousewheel is moved while the pointer is outside the popup itself, which includes the search field (or popup anchor).

You can however disable closing with the mousewheel entirely as in the attached patch. That may not be what we want here though, as it prevents scrolling the page when the search popup is open.

	Florian Quèze [:florian]
	Comment 8 • 10 years ago

(In reply to Neil Deakin from comment #7)

Thanks!

> That may not be what we want here though, as it prevents
> scrolling the page when the search popup is open.

We could verify with UX (and I will if we have a working patch), but I think Philipp will like that the panel isn't closed in that case.

The attached patch prevents the panel from blinking, but doesn't make changing the default engine with ctrl+wheel work, as the searchbox doesn't receive the even while the panel is open.

	Neil Deakin
	Comment 9 • 10 years ago

That likely requires a way to override the return value of ShouldConsumeOnMouseWheelEvent().

Two other options:

1. Add an extra value of for rolluponmousewheel that can be used to not rollup when over the textfield.
2. A bigger project may be to add a popuprollup event that occurs when the widget system thinks a rollup should occur that can be used for full control over whether the popup should hide on various events. This would prevent the need to add lots of additional attributes and special cases like this.

	Florian Quèze [:florian]
	Comment 10 • 10 years ago

Would putting a mousewheel event handler on the panel that would forward to the searchbox work? Or would the event not be dispatched to the panel at all when the mouse is outside it?

	Neil Deakin
	Comment 11 • 10 years ago

When ShouldConsumeOnMouseWheelEvent() returns true, the mousewheel event is consumed (ignored) when the mouse pointer is outside the popup.