1270494 - Autocomplete suggestions for multiple values in ruleview appear in the wrong place

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect, P3)

Description

[arni2033]

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160504043118
STR:
1. Open inspector -> rule view
2. Click on string "element {" to start editing new rule
3. Type "background", press Tab key, type "darkolivegreen"
4. Press Spacebar

AR:  Autocomplete list appears attached to the beginning of the value "darkolivegreen !important"
ER:  Autocomplete list should appear attached to the beginning of autocompleted string "!important"

This is minor regression from bug 1168246. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=dbb64aae2a239d1ae85a6591a57058ad29ab90e7&tochange=484cdaff31215bf625eb0616247a9d77ad5e1147

Comment 1

[arni2033]

Happens with every "multiple" value, e.g.:
"background: gray linear-gradient(to right, gray, gray) repeat scroll 100% 100% !important;"

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1270494 - fix autocomplete popup offset for multiple values input;

Review commit: https://reviewboard.mozilla.org/r/57882/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57882/

Comment 3

[Julian Descottes [:jdescottes]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=62ec25b8d84d

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8760200 [details]
Bug 1270494 - fix autocomplete popup offset for multiple values input;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57882/diff/1-2/

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8760200 [details]
Bug 1270494 - fix autocomplete popup offset for multiple values input;

https://reviewboard.mozilla.org/r/57882/#review55298

- The test works fine and quickly.
- The bug described in comment 0 appears to be fixed.
- When the input switches to multi-line though, there's no offset anymore. Do we not want to fix this case? Should it be a separate bug? In any case, I think we should ship this.

::: devtools/client/shared/test/browser_inplace-editor_autocomplete_offset.js:16
(Diff revision 2)
> +
> +// Test the inplace-editor autocomplete popup is aligned with the completed query.
> +// Which means when completing "style=display:flex; color:" the popup will aim to be
> +// aligned with the ":" next to "color".
> +
> +const testData = [

Please add a comment here to explain what the structure of this data array should be.

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8760200 [details]
Bug 1270494 - fix autocomplete popup offset for multiple values input;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57882/diff/2-3/

Comment 7

[Julian Descottes [:jdescottes]]

https://reviewboard.mozilla.org/r/57882/#review55298

Fixing it for the multiline case requires to change the way we calculate the left indent, because we cannot easily predict when the line breaks. I will try to play with another way to measure the indent. If it is straightforward enough I will push it here otherwise I will create a separate bug.

> Please add a comment here to explain what the structure of this data array should be.

done, also fixed in browser_inplace-editor_autocomplete_01.js & browser_inplace-editor_autocomplete_02.js

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8760200 [details]
Bug 1270494 - fix autocomplete popup offset for multiple values input;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57882/diff/3-4/

Comment 9

[Julian Descottes [:jdescottes]]

Rebased and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2466bd33e10

I logged Bug 1278789 for handling the multiline case. I'm confident this can be handled with the approach I described in Bug 1278789 comment 1, but this is out of scope of this bug so I prefer to handle this separately.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/daadc8567a11
fix autocomplete popup offset for multiple values input;r=pbro

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/daadc8567a11

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Should we consider uplifting this?

Comment 13

[Julian Descottes [:jdescottes]]

This is a minor positioning issue, not worth uplifting in my opinion.

Comment 14

[Hossain Al Ikram [:ikram] (QA Contact)]

I have reproduced this bug with Nightly 49.0a1 (2016-05-05) on Windows 7, 64 Bit!

The bug's fix is now verified on latest Nightly 50.0a1.

Build ID 	20160722030235
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0

Comment 15

[Rezaul Huque Nayeem]

Reproduced this bug in firefox nightly 49.0a1 (2016-05-05) as comment 0 with ubuntu 16.04 (64 bit)

Verified this bug as fixed with latest firefox nightly 50.0a1 (Build ID: 20160801030227)
Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0