Closed Bug 1192172 Opened 9 years ago Closed 9 years ago

[ruleview] Autocomplete popup is still visible after refreshing the page

Categories

(DevTools :: Inspector: Rules, defect)

Product:
DevTools
Component:
Inspector: Rules
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox42 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 42
Tracking Flags:
Tracking Status
firefox42 --- fixed

People

(Reporter: arni2033, Assigned: julian.descottes)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

Bug1192172.v1.patch
3.88 KB, patch
bgrins
: review+
Details | Diff | Splinter Review 
Bug 1191093 provided us this regression

STR:
1. Open data:text/html,<body>
2. Inspect <body>, click "element { }" box in ruleview
3. Type "back" as a beginning of rule name [autocomplete will appear]
4. Press F5

RESULT: (watch video)
Autocomplete is still visible and is placed above devtools.
Flags: needinfo?(julian.descottes)
Cannot investigate much from here, but I suppose the popup should now be hidden by the style inspector when navigating.

Before it was destroyed as a side effect by the markup-view.
Flags: needinfo?(julian.descottes)
As I said, I cannot currently check this. 

@arni2033 : To assess the severity, can you tell me if the popup is hidden when the user clicks outside of it ?
Or does it remain on screen forever and forces the user to kill the browser to get back to normal ?

Thanks !
Flags: needinfo?(arni2033)
(In reply to Julian Descottes from comment #2)
> the popup is hidden when the user clicks outside of it
This. (So this behavior is still better than it was before 1191093)
Flags: needinfo?(arni2033)
Thanks for the info ! 

I'll be available next week, I could try to work on this one.
In the meantime if anyone wants to take it, please do.
Interesting fact : this doesn't happen if you reload the page using CTRL/CMD+R

It looks like the function keys have a weird behavior with the autocomplete. If you focus an editor field of the RuleView and press on F1 or F2, it will :
- open the autocomplete
- write the first suggestion of the autocomplete in the field

If you keep the key pressed the first suggestion will fill the field again and again.

That's probably what happens on F5 as well and could potentially explain the issue.
@Julian
1) Nope, this happens either if I replace F5 with Ctrl+R (in my STR)
2) Ok, I'll add another function keys into my (completely different) bug 1189983. Thanks for info.
Indeed, that's a separate issue.
Refreshing page with hotkey may be a bit confusing, so I'm providing STR that don't require hotkeys:

1.* Open   data:text/html,<div onmouseover="location.reload()" style="height:50px;background:tan;">
2.  Inspect <body>, click "element { }" box in ruleview
3.  Type "back" as a beginning of rule name [autocomplete will appear]
4.* Move mouse over <div> element to refresh the page.

And let's don't discuss this much since neither of us is able to fix it at the moment.
Blocks: 1191093
See Also: 1191093 → 
(In reply to arni2033 from comment #8)
> And let's don't discuss this much since neither of us is able to fix it at
> the moment.

Huh?  It's still helpful to talk through STR and possible fixes.  Anyway, I believe we could simply hide the popup in the selectElement function, which also should fire on a document navigation.
(In reply to Brian Grinstead [:bgrins] from comment #9)
// this is true that this bug happens with any navigation, not only reloading.
> I believe we could simply hide the popup in the selectElement function, which also should fire on a document navigation.
I may be wrong here, but 1) doesn't it fire _after_ location is changed?
2) popup should be hidden _before_ location is changed? because
3) if you follow my last STR then popup is visible while you keep mouse over <div> - and no elements are selected in Inspector, because (I suppose) that function just don't have time to fire because page keeps constantly reloading.
Just pay attention to this scenario (3)
I can confirm that Brian's suggestion fixes the issue.

@arni : I tested your scenario with Brian's fix and it works fine.
(In reply to Julian Descottes from comment #11)
> I can confirm that Brian's suggestion fixes the issue.

Can you upload the patch you are using to check?  We'll want a quick regression test too but if you don't have time for that right now maybe I'll be able to add one in the meantime or it could wait until next week and get uplifted.
Simply hiding the popup in rule-view#selectElement and adding a simple mochitest.

I could not run the mochitests after my rebase (don't have the time to clobber+build). I passed them before though.

try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=f645df8e99c3
Flags: needinfo?(gabriel.luong)
Attachment #8645086 - Flags: review?(bgrinstead)
Assignee: nobody → julian.descottes
Comment on attachment 8645086 [details] [diff] [review]
Bug1192172.v1.patch

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

Nice
Attachment #8645086 - Flags: review?(bgrinstead) → review+
The failures in the try push are all unrelated, so this should be good to check in
Oops, the commit message had the wrong bug number in it (at least it was Bug 1191093, which this is fixing).  Here's the commit: https://hg.mozilla.org/integration/fx-team/rev/785195348eb8. We can manually mark this as resolved once that lands in m-c.
Flags: needinfo?(gabriel.luong)
https://hg.mozilla.org/mozilla-central/rev/785195348eb8
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
See Also: → 1200037
Component: Developer Tools: Inspector → Developer Tools: CSS Rules Inspector
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: