[Rule View] Clicking on the selector input editor will blur the editor

RESOLVED FIXED in Firefox 46

Status

defect
RESOLVED FIXED
4 years ago
10 months ago

People

(Reporter: gl, Assigned: jdescottes)

Tracking

unspecified
Firefox 46

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

4 years ago
STR:
1. Edit the Selector
2. Click on the selector input editor.
3. It blurs!

Expected:
The input caret should select wherever my click is
(Reporter)

Comment 1

4 years ago
Posted patch 1198326.patch (obsolete) — Splinter Review
Assignee: nobody → gabriel.luong
Status: NEW → ASSIGNED
Attachment #8653930 - Flags: review?(pbrosset)
(Reporter)

Comment 2

4 years ago
Posted patch 1198326.patch [1.0] (obsolete) — Splinter Review
Attachment #8653930 - Attachment is obsolete: true
Attachment #8653930 - Flags: review?(pbrosset)
Attachment #8654266 - Flags: review?(bgrinstead)
Comment on attachment 8654266 [details] [diff] [review]
1198326.patch [1.0]

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

Inplace editor changes = something might break.  Make sure you run this through the test harness and do some smoke testing with various situations in the rule view.  The only thing that looks like it might have some relation to this is https://dxr.mozilla.org/mozilla-central/source/browser/devtools/shared/inplace-editor.js#146

::: browser/devtools/shared/inplace-editor.js
@@ +244,5 @@
>    }
>    this.input.addEventListener("blur", this._onBlur, false);
>    this.input.addEventListener("keypress", this._onKeyPress, false);
>    this.input.addEventListener("input", this._onInput, false);
> +  this.input.addEventListener("dblclick", e => e.stopPropagation(), false);

Should these event listeners be removed?  For some reason they never were even though blur/keypress/input/keyup are.
Attachment #8654266 - Flags: review?(bgrinstead) → review+

Comment 4

4 years ago
Is bug 1197200 a duplicate of this? That bug is a regression between 41 and 42
See Also: → 1197200
(Reporter)

Comment 5

3 years ago
Posted patch 1198326.patch [2.0] (obsolete) — Splinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e0ea874ec56
Attachment #8654266 - Attachment is obsolete: true
Attachment #8697057 - Flags: review?(bgrinstead)
Comment on attachment 8697057 [details] [diff] [review]
1198326.patch [2.0]

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

Works for me if existing tests pass. Can you please add a test case in one of the existing inplace editor tests?
Attachment #8697057 - Flags: review?(bgrinstead) → feedback+

Updated

3 years ago
Blocks: 1197200

Updated

3 years ago
See Also: 1197200
(Reporter)

Updated

3 years ago
Assignee: gl → jdescottes
(Assignee)

Comment 7

3 years ago
Posted patch bug1198326.v3.patch (obsolete) — Splinter Review
Readapted the patch from Gabriel and added one test case.

Try : https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1e18cd95cac
Attachment #8697057 - Attachment is obsolete: true
Attachment #8709608 - Flags: review?(bgrinstead)
Comment on attachment 8709608 [details] [diff] [review]
bug1198326.v3.patch

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

Nice, seems to fix it for me!

::: devtools/client/inspector/rules/test/browser_rules_edit-selector-click.js
@@ +3,5 @@
> + http://creativecommons.org/publicdomain/zero/1.0/ */
> +
> +"use strict";
> +
> +// Testing selector inplace-editor behaviors in the rule-view with invalid

This comment doesn't match up with what the test is doing
Attachment #8709608 - Flags: review?(bgrinstead) → review+
(Assignee)

Comment 9

3 years ago
Thanks for the catch!

Carry over r+ from previous review.
Attachment #8709608 - Attachment is obsolete: true
Attachment #8709748 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/41efc3cbf8d6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1197200
Duplicate of this bug: 1258653

Comment 14

3 years ago
[bugday-20160323]

Status: RESOLVED,FIXED --> VERIFIED

Component: 
Name 	        Firefox
Version 	46.0b2
Build ID 	20160314144540
Update Channel 	beta
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS              Windows 7 SP1 x86_64

Actual Results: 
As expected

Expected Results: 
The input caret should select wherever my click is.

Updated

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