Autocomplete popup's isOpen getter is too strict.

RESOLVED FIXED in Firefox 29

Status

RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: Optimizer, Assigned: Optimizer)

Tracking

unspecified
Firefox 29
x86_64
Windows 7
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 8363753 [details] [diff] [review]
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.
Attachment #8363753 - Flags: review?(mihai.sucan)
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.
Attachment #8363753 - Flags: review?(mihai.sucan) → review+
(Assignee)

Comment 2

5 years ago
(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.
(Assignee)

Comment 3

5 years ago
landed in fx-team 

https://hg.mozilla.org/integration/fx-team/rev/cbc8854278fb
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cbc8854278fb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.