Bug 148351

Summary:

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

Product:

WebKit

Reporter:

Timothy Hatcher <timothy>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, commit-queue, graouts, hi, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

147563

Attachments:

Description	Flags
Video	none
Patch	bburg: review+, bburg: commit-queue-
Patch	none

Timothy Hatcher

Reported 2015-08-21 20:33:55 PDT

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.

Attachments
Video (6.94 MB, video/quicktime) 2015-08-21 20:33 PDT, Timothy Hatcher	no flags	Details
Patch (5.57 KB, patch) 2015-09-11 22:30 PDT, Devin Rousso	bburg: review+ bburg: commit-queue-	Details Formatted Diff Diff
Patch (5.45 KB, patch) 2015-09-15 20:12 PDT, Devin Rousso	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2015-09-08 16:05:26 PDT

<rdar://problem/22619595>

Devin Rousso

Comment 2 2015-09-11 22:30:59 PDT

Created attachment 261045 [details] Patch

Blaze Burg

Comment 3 2015-09-15 10:43:30 PDT

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

Comment 4 2015-09-15 20:05:47 PDT

(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

Comment 5 2015-09-15 20:12:18 PDT

Created attachment 261289 [details] Patch

WebKit Commit Bot

Comment 6 2015-09-15 21:22:36 PDT

Comment on attachment 261289 [details] Patch Clearing flags on attachment: 261289 Committed r189845: <http://trac.webkit.org/changeset/189845>

WebKit Commit Bot

Comment 7 2015-09-15 21:22:40 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.