Bug 148351 – Web Inspector: Picking unchanged for font-size does not reset back to the unchanged value

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

[Timothy Hatcher]

Created attachment 259704 [details]
Video

Steps:

1)Change font-size.
2)Pick unchanged from the menu

Results: Stays at the edited value. Note the rule icons reverts to the unchanged icon.
Expected: Reverts to the original computed value

See video.

[Radar WebKit Bug Importer]

<rdar://problem/22619595>

[Devin Rousso]

Created attachment 261045 [details]
Patch

[Blaze Burg]

Comment on attachment 261045 [details]
Patch

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

r=me. Please change the flag setter's name, though.

I would like us to work towards storing less state in the visual style editor's DOM elements.

> Source/WebInspectorUI/UserInterface/Views/VisualStyleNumberInputBox.js:115
> +            if (value || this._updatedValues.placeholder)

There are lots of places where we have fallback values, can these be extracted into well-named getters?

> Source/WebInspectorUI/UserInterface/Views/VisualStyleNumberInputBox.js:235
> +    _markNumberInputEditable(flag)

This should match the name of the flag, i.e., this._setNumberInputIsEditable(flag). There's no reason for it to differ, and it reads better.

[Devin Rousso]

(In reply to comment #3)
> Comment on attachment 261045 [details]

> I would like us to work towards storing less state in the visual style
> editor's DOM elements.
I would agree, but I think that that should be its own bug.  I can modify all the visual style classes to use this methodology and will do so soon (my homework sucks).

> > Source/WebInspectorUI/UserInterface/Views/VisualStyleNumberInputBox.js:115
> > +            if (value || this._updatedValues.placeholder)
> 
> There are lots of places where we have fallback values, can these be
> extracted into well-named getters?

I think that, for now, it is easier and clearer to use these fallbacks.  I've thought a bit about how to move away from storing the state in the DOM and I think that that change would resolve many of these cases.

[Devin Rousso]

Created attachment 261289 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 261289 [details]
Patch

Clearing flags on attachment: 261289

Committed r189845: <http://trac.webkit.org/changeset/189845>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.