Closed
Bug 913955
Opened 11 years ago
Closed 11 years ago
Style Inspector CSS autocomplete breaks manual entry in inplace editor
Categories
(DevTools :: Inspector, defect)
Tracking
(firefox30 fixed, firefox31 fixed)
RESOLVED
FIXED
Firefox 31
People
(Reporter: cheesypoof2020, Assigned: Optimizer)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
14.76 KB,
image/png
|
Details | |
4.32 KB,
patch
|
miker
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
The style inspector's autocomplete can butcher one's attempt to manually enter a CSS value. Let's say I wish to add a 'background-clip' property to an element... I click on some of the whitespace in the style inspector panel, type 'background-clip', and hit enter. From this point if I type (quite rapidly) 'content', it produces 'content-boxt-boxent-box'. I have also seen 'content-boxnt-boxnt-box', 'content-box-box-box-box-box', and 'content-boxnt-box'. I can also produce similar behavior for 'display' when I type 'inline'. The common factor seems to be a '-' in the list of potential property values.
Couldn't reproduce on latest Aurora (20131007004003) on Ubuntu 13.04 and Win8 32-bit. Can you reproduce this issue with a clean profile on latest Nightly? Thank you http://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles
I was able to repro it randomly when the bug has been filed, but not now.
Flags: needinfo?(cheesypoof2020)
Reporter | ||
Comment 3•11 years ago
|
||
I am still able to reproduce this issue, including with a new Nightly profile. Nevertheless, it does require me to type 'content' very quickly to see this behavior.
Flags: needinfo?(cheesypoof2020)
Comment 4•11 years ago
|
||
I was able to reproduce this in bug 934695: If the css value is "red" and I move my input selection to "r|ed", and continue typing "e", then the autocomplete will fill in "reded". I can work on this after 934695. Relevant section of the code is in inplace-editor.js#_maybeSuggestCompletion. For the query, we should check if there is any characters after the input selection, and if there is halt the autocompletion.
Comment 5•11 years ago
|
||
Work in progress. Still need to consider edge cases such as having a space after the input selection, eg) "color : | red", and test cases.
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 7•11 years ago
|
||
-Halts autosuggestion if there are any non-space character in front of the caret
Attachment #828249 -
Attachment is obsolete: true
Attachment #830004 -
Flags: review?(scrapmachines)
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 830004 [details] [diff] [review] 913955.patch Review of attachment 830004 [details] [diff] [review]: ----------------------------------------------------------------- I am afraid that this issue is a bit tricky. There are two things happening here : - User clicking in the middle of a word and starting to type, which leads to autocopmletion being triggered and thus things like "pad[ding]ing" happening (things in [] are selected) - User typing too fast that the autocompletion is not able to cope up and thus things like in the first comment's screenshot happen. Here we cannot simply stop autocompleting as the completion here automatically types in the first entry as you type, so if you are typing "pad" , "ding" will automatically be added (like "pad[ding]" ) as you type. Thus, you will *always* have a character after the cursor without a space. So we have to do two things here : - Do not autocomplete where there is non-selected text after the cursor. i.e. selectionStart == selectionEnd && no space after selectionStart - Somehow sync up suggesting autocopmletion and the user typing rapidly so that no race conditions like in #1's screenshot happen.
Attachment #830004 -
Flags: review?(scrapmachines) → review-
Comment 9•11 years ago
|
||
Not actively working on this bug in case anyone wants to pick it up.
Comment 10•11 years ago
|
||
I have the same issue as in Comment 1 somewhat often, and it is a bad experience. > - Do not autocomplete where there is non-selected text after the cursor. i.e. selectionStart == selectionEnd && no space after selectionStart Even if we just caught this particular type of error case and closed the popup it would be better than having stuff inserted into the middle of your word. I also wonder if handling this would also fix the race condition problem, if it is just sporadically causing this same issue when typing too fast. I think a good first fix would be the same as Attachment 830004 [details] [diff] but with the added check that the selection is collapsed, which would at least fix the case in Comment 4.
Updated•11 years ago
|
Blocks: DevToolsPaperCuts
Summary: Style Inspector CSS autocomplete breaks manual entry → Style Inspector CSS autocomplete breaks manual entry in inplace editor
Assignee | ||
Comment 11•11 years ago
|
||
patches welcome :)
Assignee | ||
Comment 12•11 years ago
|
||
This fixes both the issues happening on slow machines: 1) Race condition when the timeout for input "r" is called when the input is already "re", thus creating situations like "reded" 2) When the user is typing something in between already entered text like "re|d" where | is the cursor. Added a test to check for the second str try push : https://tbpl.mozilla.org/?tree=Try&rev=f97a90e370f6
Assignee: nobody → scrapmachines
Attachment #830004 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8391976 -
Flags: review?(mratcliffe)
Comment 13•11 years ago
|
||
Comment on attachment 8391976 [details] [diff] [review] patch Review of attachment 8391976 [details] [diff] [review]: ----------------------------------------------------------------- Very simple fix for an annoying problem, r+
Attachment #8391976 -
Flags: review?(mratcliffe) → review+
Assignee | ||
Comment 14•11 years ago
|
||
landed in fx-team : https://hg.mozilla.org/integration/fx-team/rev/c354ec771e52
Whiteboard: [fixed-in-fx-team]
Comment 15•11 years ago
|
||
Weird issue with this (or maybe Bug 912189): * When I press cmd+a it inserts -moz-animation into the front of the editor regardless of where the cursor is or what is already in the box. So if I press three times I see -moz-animation-moz-animation-moz-animation. Same thing happens with ctrl+a (on mac), but it only inserts it once.
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #15) > Weird issue with this (or maybe Bug 912189): > > * When I press cmd+a it inserts -moz-animation into the front of the editor > regardless of where the cursor is or what is already in the box. So if I > press three times I see -moz-animation-moz-animation-moz-animation. Same > thing happens with ctrl+a (on mac), but it only inserts it once. ugh. This was an already existing issue/defect, which got exposed by Bug 912189 now as empty editors could also suggest now. Path in bug 987870 fixes this.
https://hg.mozilla.org/mozilla-central/rev/c354ec771e52
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Assignee | ||
Comment 19•11 years ago
|
||
its okay, as long as bug 987870 is also uplifted
Flags: needinfo?(scrapmachines)
Assignee | ||
Comment 20•11 years ago
|
||
Comment on attachment 8391976 [details] [diff] [review] patch [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
Attachment #8391976 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #8391976 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
status-firefox30:
--- → affected
status-firefox31:
--- → fixed
Updated•11 years ago
|
Comment 22•10 years ago
|
||
Looks like this landed with tests. Please correct if I am mistaken.
Flags: in-testsuite+
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•