Bug 179831 – Web Inspector: Styles Redesign: pressing delete in empty value field should focus on name field

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Chris Chiera]

In Chrome if you enter say something like w and tab to auto complete, and it autocompletes something that wasn't desired you can easily clicked Delete/Backspace and it will automatically highlight that rule such as "white-space" allowing you to instantly type in a new word such as "width". In Safari Preview 44, after it autocompletes, backspace/delete does nothing. If I clicked on the word "white-space" in attempt to change it it simple deletes it all together and have to start all over again.

In older versions such as the current stable version of Safari, it works better than 44, in that backspace allows you to go back, but it just deletes the ":" and then each letter, rather than automatically deleting the ":" and automatically selecting the entire word. 

Other than a couple small bugs like this, version 44, is an incredible improvement to the inspector and resolves countless little bugs/issues in regards to editing styles/html. We all appreciate all your incredible hard work.

[Nikita Vasilyev]

The behavior I'm seeing in the new styles sidebar matches Chrome. Could you provide the exact steps to reproduce the problem? We like using this steps/actual/expected template:

Steps:
1. Add a new CSS property.
2. Type "w".
3. Press Tab.

Actual:
"white-space" property name is selected. Focus moves to property name.

Expected:
???

[Chris Chiera]

Sure thing. So in your example you mentioning simply autocompleting (which works perfectly), this is in regards to what happens when you backspace/delete after autocompleting.

So here is Safari Preview 44. On say apple.com. I add new rule and auto complete it like normal. Then I wish to change that if say it automcompleted the wrong word. Back when clicking delete, nothing happens: http://share.heavymark.com/1U3i3p0v3v0F

Now here is in Chrome stable on apple.com where it works as expected: http://share.heavymark.com/3S2R0s0v3n0t There after auto completing I can go back easily by clicking delete. 

In the current stable version of Safari it lets you go back (so, better than 44, but not as good as Chrome since it does per character rather than highlighting the whole word: http://share.heavymark.com/2P3h3p171K3c

If 44 is currently working like the chrome video above let me know and I can try reinstalling or trying on another computer. Currently using latest MBP with touchbar and latest stable version of MacOS. Hope this helps.

[Nikita Vasilyev]

I see. I didn't know that Chrome has a special case for newly added properties! If I focus on the existing property value and press delete, it doesn't move to the property name.

[Radar WebKit Bug Importer]

<rdar://problem/35626976>

[Devin Rousso]

Created attachment 328316 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 328316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328316&action=review

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:270
> +        else if (textField === this._valueTextField)
> +            this._nameTextField.startEditing();

So Chrome's behavior is to only have backspace go from the value -> name when its a new property. I think this is so that when you start editing an existing property value and you select its contents and hit backspace, it only clears the value but doesn't also jump to the name property. What is the behavior after this patch is applied?

[Devin Rousso]

Comment on attachment 328316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328316&action=review

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:270
>> +            this._nameTextField.startEditing();
> 
> So Chrome's behavior is to only have backspace go from the value -> name when its a new property. I think this is so that when you start editing an existing property value and you select its contents and hit backspace, it only clears the value but doesn't also jump to the name property. What is the behavior after this patch is applied?

Interesting.  The behavior here doesn't distinguish between existing and new properties.  Personally, I find this more useful, as it allows me to delete properties a lot easier, but I could go either way.  How do you want to proceed?

[Timothy Hatcher]

Comment on attachment 328316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328316&action=review

> Source/WebInspectorUI/ChangeLog:12
> +        * UserInterface/Views/SpreadsheetStyleProperty.js:

Oi, spreadsheet. Hmm.

>>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:270
>>> +            this._nameTextField.startEditing();
>> 
>> So Chrome's behavior is to only have backspace go from the value -> name when its a new property. I think this is so that when you start editing an existing property value and you select its contents and hit backspace, it only clears the value but doesn't also jump to the name property. What is the behavior after this patch is applied?
> 
> Interesting.  The behavior here doesn't distinguish between existing and new properties.  Personally, I find this more useful, as it allows me to delete properties a lot easier, but I could go either way.  How do you want to proceed?

I find Chrome's behavior odd, and sounds like it could be a bug. I agree with Devin, that pressing delete enough should jump from value to name and continue editing there.

[Timothy Hatcher]

Maybe the right solution would be to allow editing existing values, pressing backspace clears and stays focused in the value, but a second backspace jumps to the name and selects it. That way if you do just want to clear the value and type a new one it works as expected, but you can get to the value too.

In my mind backspace/delete would be like a destructive reverse tab. I should be able to start at the value of the last property and holding delete, end up focused on a blank single property name after deleting through the lines of a whole rule.

[Timothy Hatcher]

Comment on attachment 328316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328316&action=review

> Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js:266
> +            if (!this.value) {

I think this does implement what I said in my last comment on how I think it should work. An initial backspace would happen when this has a value and it would just fall through and clear it as an editing operation.

[Devin Rousso]

Comment on attachment 328316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328316&action=review

>> Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js:266
>> +            if (!this.value) {
> 
> I think this does implement what I said in my last comment on how I think it should work. An initial backspace would happen when this has a value and it would just fall through and clear it as an editing operation.

Correct!  It will only jump back to the name (by calling `spreadsheetTextFieldDidBackspace`) if the editor has no content.  It would require two backspaces, the first of which to delete the existing value, to move between editors.

[WebKit Commit Bot]

Comment on attachment 328316 [details]
Patch

Clearing flags on attachment: 328316

Committed r225570: <https://trac.webkit.org/changeset/225570>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

(In reply to Timothy Hatcher from comment #9)
> Maybe the right solution would be to allow editing existing values, pressing
> backspace clears and stays focused in the value, but a second backspace
> jumps to the name and selects it. That way if you do just want to clear the
> value and type a new one it works as expected, but you can get to the value
> too.

That was what I was thinking as well.

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #11)
> Comment on attachment 328316 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=328316&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js:266
> >> +            if (!this.value) {
> > 
> > I think this does implement what I said in my last comment on how I think it should work. An initial backspace would happen when this has a value and it would just fall through and clear it as an editing operation.
> 
> Correct!  It will only jump back to the name (by calling
> `spreadsheetTextFieldDidBackspace`) if the editor has no content.  It would
> require two backspaces, the first of which to delete the existing value, to
> move between editors.

I like this!

Nit: please retitle the bug before uploading the patch next time.