Along the lines of <https://bugs.webkit.org/show_bug.cgi?id=148678>, if invalid CSS (meaning that the value is not valid, not that the property is prefixed) already exists, instead of silently failing and showing the computed value, display an error icon explaining that the current value is invalid (similar to what happens if "display: asd;" is entered into the Rules sidebar, but with an Error icon instead of a Warning icon).
<rdar://problem/24424025>
Created attachment 270302 [details] Patch
Comment on attachment 270302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270302&action=review > Source/WebInspectorUI/UserInterface/Views/VisualStyleNumberInputBox.js:219 > + WebInspector.VisualStylePropertyEditor.prototype.__lookupSetter__("specialPropertyPlaceholderElementText").call(this, text); So I realize that this is not the best way to do this, but since I can't call "super.specialPropertyPlaceholderElementText = text", I wasn't sure what to do. I figured that this solution was better than making an inner function like "this._setSpecialPropertyPlaceholderElementText(text)" since the function's body was so minimal. Is it preferred to do it this way or using the inner function?
Please upload a screenshot with the change.
Comment on attachment 270302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270302&action=review >> Source/WebInspectorUI/UserInterface/Views/VisualStyleNumberInputBox.js:219 >> + WebInspector.VisualStylePropertyEditor.prototype.__lookupSetter__("specialPropertyPlaceholderElementText").call(this, text); > > So I realize that this is not the best way to do this, but since I can't call "super.specialPropertyPlaceholderElementText = text", I wasn't sure what to do. I figured that this solution was better than making an inner function like "this._setSpecialPropertyPlaceholderElementText(text)" since the function's body was so minimal. Is it preferred to do it this way or using the inner function? I had no idea this was a thing. I guess it's ok since a FIXME is above it.
Created attachment 270417 [details] [Image] After patch is applied There is also a tooltip on the Error icon that says: The value '%s' is not supported for this property.
Comment on attachment 270302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270302&action=review > Source/WebInspectorUI/ChangeLog:8 > + Please include summary blurb here. > Source/WebInspectorUI/ChangeLog:33 > + Before assigning the value of the property, if found, to the editor, This sentence is awkwards.
Created attachment 270603 [details] Patch
Comment on attachment 270603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270603&action=review > Source/WebInspectorUI/UserInterface/Views/VisualStylePropertyEditor.js:294 > + this._warningElement.title = WebInspector.UIString("The value '%s' is not supported for this property.").format(propertyText); Should use curly quotes (“”) not single quotes.
Comment on attachment 270603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270603&action=review >> Source/WebInspectorUI/UserInterface/Views/VisualStylePropertyEditor.js:294 >> + this._warningElement.title = WebInspector.UIString("The value '%s' is not supported for this property.").format(propertyText); > > Should use curly quotes (“”) not single quotes. Really? OK I have been using ('') incorrectly for a while then. All of the warning icon's tooltips (both the Rules and Visual sidebars) use '%s'. Would you rather me make another patch to resolve all of these issues, or just fix this one?
Comment on attachment 270603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270603&action=review >>> Source/WebInspectorUI/UserInterface/Views/VisualStylePropertyEditor.js:294 >>> + this._warningElement.title = WebInspector.UIString("The value '%s' is not supported for this property.").format(propertyText); >> >> Should use curly quotes (“”) not single quotes. > > Really? OK I have been using ('') incorrectly for a while then. All of the warning icon's tooltips (both the Rules and Visual sidebars) use '%s'. Would you rather me make another patch to resolve all of these issues, or just fix this one? You can fix them in a follow up.
Comment on attachment 270603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270603&action=review >>>> Source/WebInspectorUI/UserInterface/Views/VisualStylePropertyEditor.js:294 >>>> + this._warningElement.title = WebInspector.UIString("The value '%s' is not supported for this property.").format(propertyText); >>> >>> Should use curly quotes (“”) not single quotes. >> >> Really? OK I have been using ('') incorrectly for a while then. All of the warning icon's tooltips (both the Rules and Visual sidebars) use '%s'. Would you rather me make another patch to resolve all of these issues, or just fix this one? > > You can fix them in a follow up. Ok will do. <https://bugs.webkit.org/show_bug.cgi?id=153891>
Comment on attachment 270603 [details] Patch Clearing flags on attachment: 270603 Committed r196146: <http://trac.webkit.org/changeset/196146>
All reviewed patches have been landed. Closing bug.