WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
146859
Web Inspector: Warning icon tooltip for numbers with no units could be improved
https://bugs.webkit.org/show_bug.cgi?id=146859
Summary
Web Inspector: Warning icon tooltip for numbers with no units could be improved
Timothy Hatcher
Reported
2015-07-10 16:01:58 PDT
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).
Attachments
Screenshot
(61.45 KB, image/png)
2015-07-10 16:01 PDT
,
Timothy Hatcher
no flags
Details
Patch
(3.81 KB, patch)
2015-07-10 19:19 PDT
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(3.82 KB, patch)
2015-07-10 23:56 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-07-10 16:03:01 PDT
<
rdar://problem/21776057
>
Devin Rousso
Comment 2
2015-07-10 19:19:05 PDT
Created
attachment 256637
[details]
Patch
Timothy Hatcher
Comment 3
2015-07-10 19:58:21 PDT
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+$/?
Devin Rousso
Comment 4
2015-07-10 23:56:20 PDT
Created
attachment 256650
[details]
Patch
Timothy Hatcher
Comment 5
2015-07-11 00:56:20 PDT
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?
Timothy Hatcher
Comment 6
2015-07-11 00:57:17 PDT
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.
WebKit Commit Bot
Comment 7
2015-07-11 01:43:43 PDT
Comment on
attachment 256650
[details]
Patch Clearing flags on attachment: 256650 Committed
r186708
: <
http://trac.webkit.org/changeset/186708
>
WebKit Commit Bot
Comment 8
2015-07-11 01:43:47 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.
Top of Page
Format For Printing
XML
Clone This Bug