987870 - [rule-view] Cursor should move between inplace editor auto-opened suggestions

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Brian Grinstead [:bgrins]]

STR:

Type "color:" into a new rule property
A dropdown opens with a suggested list of colors
Press 'down'

Expected that pressing arrow keys moves around the list.  Won't move around the list until you start typing a character and one of the results becomes selected.

Comment 1

[Girish Sharma [:Optimizer]]

Attachment: patch

Fixes this and https://bugzilla.mozilla.org/show_bug.cgi?id=913955#c15

Comment 2

[Brian Grinstead [:bgrins]]

Comment on attachment 8396600 [details] [diff] [review]
patch

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

Just need to replace accelKey with metaKey and I'll r+ with a green try

::: browser/devtools/shared/inplace-editor.js
@@ +872,5 @@
>          aEvent.keyCode === Ci.nsIDOMKeyEvent.DOM_VK_RIGHT) {
>        if (this.popup && this.popup.isOpen) {
>          this.popup.hidePopup();
>        }
> +    } else if (!cycling && !aEvent.accelKey && !aEvent.altKey &&

accelKey -> metaKey

::: browser/devtools/styleinspector/test/browser_bug894376_css_value_completion_new_property_value_pair.js
@@ +20,5 @@
>  //    selectedIndex of the popup,
>  //    total items in the popup
>  //  ]
>  let testData = [
> +  ["a", {accelKey: true, ctrlKey: true}, "", -1, 0],

accelKey -> metaKey

@@ +87,5 @@
>        checkState();
>      });
>    }
> +  else if (/(right|back_space|escape|return)/ig.test(key) ||
> +           (modifiers.accelKey || modifiers.ctrlKey)) {

accelKey -> metaKey

Comment 3

[Girish Sharma [:Optimizer]]

Attachment: patch v2

Not converting to metaKey in synthesizeKey as there it wants accelKey to simulate cmd.

pushed to try : https://tbpl.mozilla.org/?tree=Try&rev=caba97f4c351

Comment 4

[Brian Grinstead [:bgrins]]

Found another weird bug here (with and without patch applied).  If I type "color:" and the list opens up then select one of them with my mouse the property disappears nothing gets applied.

Comment 5

[Girish Sharma [:Optimizer]]

That is a regression from a long time back when some output parser refactoring was done. I don;t know if I have a bug on file though.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Girish Sharma [:Optimizer] from comment #5)
> That is a regression from a long time back when some output parser
> refactoring was done. I don;t know if I have a bug on file though.

Comparing with Nightly, I think Bug 912189 has aggravated this problem.  In nightly, after selecting one with the mouse it picks it but the popup sticks around - and if you do it a couple of times some weird stuff happens.  Now if you pick with the mouse then everything disappears and the property is deleted.

We need to either fix this problem here or make sure that a bug is filed that will land along with 912189 to fix this one.

Comment 7

[Girish Sharma [:Optimizer]]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> (In reply to Girish Sharma [:Optimizer] from comment #5)
> > That is a regression from a long time back when some output parser
> > refactoring was done. I don;t know if I have a bug on file though.
> 
> Comparing with Nightly, I think Bug 912189 has aggravated this problem.  In
> nightly, after selecting one with the mouse it picks it but the popup sticks
> around - and if you do it a couple of times some weird stuff happens.  Now
> if you pick with the mouse then everything disappears and the property is
> deleted.
> 
> We need to either fix this problem here or make sure that a bug is filed
> that will land along with 912189 to fix this one.

This bug already clubs 2 separate issues. Followup is the right way imo.

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Girish Sharma [:Optimizer] from comment #7)
> (In reply to Brian Grinstead [:bgrins] from comment #6)
> > (In reply to Girish Sharma [:Optimizer] from comment #5)
> > > That is a regression from a long time back when some output parser
> > > refactoring was done. I don;t know if I have a bug on file though.
> > 
> > Comparing with Nightly, I think Bug 912189 has aggravated this problem.  In
> > nightly, after selecting one with the mouse it picks it but the popup sticks
> > around - and if you do it a couple of times some weird stuff happens.  Now
> > if you pick with the mouse then everything disappears and the property is
> > deleted.
> > 
> > We need to either fix this problem here or make sure that a bug is filed
> > that will land along with 912189 to fix this one.
> 
> This bug already clubs 2 separate issues. Followup is the right way imo.

Fine with me - I've filed Bug 988001

Comment 9

[Brian Grinstead [:bgrins]]

Let's wait for try to return before checking in to make sure our meta/accel keys are working across all platforms

Comment 10

[Girish Sharma [:Optimizer]]

green enough. (random unrelated oranges)

Comment 11

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

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/07bc2edf7b9d

Comment 13

[Girish Sharma [:Optimizer]]

Comment on attachment 8396682 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 912189
User impact if declined: This bug fixes an issue introduced by 912189. Also, this patch is needed as a part of a bigger queue so as to uplift the chunks-by-dir feature of tests to aurora. see https://tbpl.mozilla.org/?tree=Try&rev=bfee331a418c
Testing completed (on m-c, etc.): mc
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dd5fc2e348bb