Closed Bug 988001 Opened 6 years ago Closed 6 years ago

[rule view] Clicking outside of an empty value inplace editor makes the property disappear

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(firefox30 fixed, firefox31 fixed)

RESOLVED FIXED
Firefox 31
Tracking Status
firefox30 --- fixed
firefox31 --- fixed

People

(Reporter: bgrins, Assigned: Optimizer)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

If I type "color:" and the list opens up then select one of them with my mouse the property disappears and nothing gets applied.

Comparing with Nightly, I think Bug 912189 has aggravated this problem.  In nightly, after selecting one with the mouse it picks the selected item but the popup sticks around.  Now if you pick with the mouse then everything disappears and the property is deleted.
OS: Mac OS X → All
Priority: -- → P1
Hardware: x86 → All
This is not actually the popup's fault , now that I tried it.

I don't have bug 912189 in my nightly yet and I can still reproduce this without the popup.

type "color:", you are automatically taken to next blank input box. Just click anywhere outside, the whole property disappears.

I am assuming something else caused this.
No longer depends on: 912189
Summary: [rule view] Selecting item from autocomplete list with mouse causes property to be removed → [rule view] Clicking outside of an empty value inplace editor makes the property disappear
Pressing escape also does the same.
Attached patch patch v0.1 (obsolete) — Splinter Review
So this makes the popup click selection work again.

try : https://tbpl.mozilla.org/?tree=Try&rev=8b16979b8fb1
Assignee: nobody → scrapmachines
Status: NEW → ASSIGNED
Attachment #8409077 - Flags: review?(bgrinstead)
Comment on attachment 8409077 [details] [diff] [review]
patch v0.1

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

Yay! r+ with green try and minor comments addressed

::: browser/devtools/shared/inplace-editor.js
@@ +787,2 @@
>        let label, preLabel;
> +      console.log(this.popup.selectedIndex, this._selectedIndex)

Extra log statement

@@ +820,5 @@
>        };
>        this.popup._panel.addEventListener("popuphidden", onPopupHidden);
>        this.popup.hidePopup();
> +      // We would like to apply this value and commit it for rule view. So exit
> +      // only for markup view.

The comment is confusing.  When is the popup loaded in the markup view when it is *not* for inline styles (which I believe would be CSS_MIXED)?
Attachment #8409077 - Flags: review?(bgrinstead) → review+
(In reply to Brian Grinstead [:bgrins] from comment #4)
> Comment on attachment 8409077 [details] [diff] [review]
> patch v0.1
> 
> Review of attachment 8409077 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yay! r+ with green try and minor comments addressed
> 
> ::: browser/devtools/shared/inplace-editor.js
> @@ +787,2 @@
> >        let label, preLabel;
> > +      console.log(this.popup.selectedIndex, this._selectedIndex)
> 
> Extra log statement

erf.

> @@ +820,5 @@
> >        };
> >        this.popup._panel.addEventListener("popuphidden", onPopupHidden);
> >        this.popup.hidePopup();
> > +      // We would like to apply this value and commit it for rule view. So exit
> > +      // only for markup view.
> 
> The comment is confusing.  When is the popup loaded in the markup view when
> it is *not* for inline styles (which I believe would be CSS_MIXED)?

CSS_MIXED Is the contentType of inplace editor for markup view. The difference here is that the markup view's editor does not show live previews. So we do not call ._apply() for that.

I will try to improve the comment wrt that.
try was green. landed in fx-team : https://hg.mozilla.org/integration/fx-team/rev/ee7632b3d59f
Whiteboard: [fixed-in-fx-team]
fixed comment and removed console log.
Attachment #8409077 - Attachment is obsolete: true
Attachment #8409298 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ee7632b3d59f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Comment on attachment 8409298 [details] [diff] [review]
patch that landed

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 912189 made this bug much easier to trigger. Bug 912189 is in aurora, thus this request.
User impact if declined: not able to insert suggestions via popup click
Testing completed (on m-c, etc.): mc, fx-team
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none
Attachment #8409298 - Flags: approval-mozilla-aurora?
Attachment #8409298 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: in-testsuite?
Whiteboard: [qa-]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.