962652 - Autocomplete popup's isOpen getter is too strict.

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Girish Sharma [:Optimizer]]

Attachment: fix

basically, it just checks for state == "open".

The more I use inspector's popup and from what I have learned from writing all the tests which check for popup's open/close state and its content, I have come to realize that `state == "showing"` is also an open state.

In "showing" state, the popup works as usual, its inner elements are available to DOM and etc. Just that the popup is not completely drawn/visible on the screen.

For fast typing users, we should not wait for the popup to completely open before they can choose the first result and autocomplete. This is more evident in case of slow computers.

I am already custom checking the popup._panel.state in all the tests because without that, the tests will never pass.

The attached patch makes the suggested changes.

try run : https://tbpl.mozilla.org/?tree=Try&rev=830c9e2a7d3f

PS: component is framework as the popup is used in many tools now.

Comment 1

[Mihai Sucan [:msucan]]

Comment on attachment 8363753 [details] [diff] [review]
fix

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

This is interesting to learn. I believed that being stricter might help avoiding failures when accessing various DOM elements. r+ with several green try runs.

Make sure you fix the patch commit message.

Comment 2

[Girish Sharma [:Optimizer]]

(In reply to Mihai Sucan [:msucan] from comment #1)
> Comment on attachment 8363753 [details] [diff] [review]
> fix
> 
> Review of attachment 8363753 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is interesting to learn. I believed that being stricter might help
> avoiding failures when accessing various DOM elements. r+ with several green
> try runs.

Yup, interstingly, the style editor test I added in bug 717369 simulates typing "fo" and then pressing tab. Since everything is so fast, without the loose check for popup open, it would just think that popup is closed and add spaces instead.


> Make sure you fix the patch commit message.

Yup will send a couple of more. with more reties.

Comment 3

[Girish Sharma [:Optimizer]]

landed in fx-team 

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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