| Summary: | Web Inspector: Warning icon tooltip for numbers with no units could be improved | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||||
| Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 256637 [details]
Patch
Comment on attachment 256637 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256637&action=review > Source/WebInspectorUI/ChangeLog:12 > + (WebInspector.CSSStyleDeclarationTextEditor.prototype._createTextMarkerForPropertyIfNeeded): > + If the property's value is incorrect and is comprised of only numbers, that must mean that the value needs > + to have units (like "px") after the number. Added another warning icon case to support this scenario. I guess this could be too generic, as px might not be the right unit (% or deg come to mind). But I can't think of any property off the top of my head. > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1076 > + if (/\d+$/.test(property.value)) { /^\d+$/? Created attachment 256650 [details]
Patch
Comment on attachment 256650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256650&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1076 > + if (/^(?:\d+)$/.test(property.value)) { Why does it need to be a no capturing group? Comment on attachment 256650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=256650&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:1079 > + title: WebInspector.UIString("The value '%s' needs units.\nClick to add 'px' to the value.").format(property.value), All the single quotes should be changed to curly (“”) double quotes. You should do that in a follow up patch. Comment on attachment 256650 [details] Patch Clearing flags on attachment: 256650 Committed r186708: <http://trac.webkit.org/changeset/186708> All reviewed patches have been landed. Closing bug. |
Created attachment 256621 [details] Screenshot The warning says a number (other than zero) is not supported for properties like margin-top. The action and tooltip for the warning icon offer to delete the value and autocomplete. We might want to suggest adding "px" to the number to make it valid (since px is assumed for numbers in non-strict mode).