Summary: | Web Inspector: CSS editing breaks when entering "color: rgb(1" | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Pavel Feldman <pfeldman> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Pavel Feldman
2011-04-29 02:34:56 PDT
Created attachment 91656 [details]
Patch
Comment on attachment 91656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91656&action=review > LayoutTests/inspector/styles/styles-add-invalid-property.html:31 > + treeElement.valueElement.textContent = "rgb("; This should be "rgb(1", as per the failing case in the reported issue. Created attachment 91677 [details]
Patch
Comment on attachment 91677 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91677&action=review These are the primary issues I noticed. > LayoutTests/inspector/styles/styles-commit-editing.html:25 > + treeElement.valueElement.textContent = "rgb(1"; doesn't the following sequence work? treeElement.valueElement.textContent = "rgb("; treeElement.valueElement.dispatchEvent(InspectorTest.createKeyEvent("U+0031")); This could save us a fortune (and a ForTest method). > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1644 > + } else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code || event.keyIdentifier === "Escape") A better approach is to fix this to check a single event.keyIdentifier === "U+001B". This way we won't have to add two hacks. > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1941 > + return typeof this.originalPropertyText === "string"; If you don't use any other type than "string", why is the original condition changed? > Source/WebCore/inspector/front-end/inspector.js:1555 > + a new blank line might not be necessary Created attachment 91684 [details]
Patch
Created attachment 91685 [details]
Patch
(In reply to comment #4) > (From update of attachment 91677 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=91677&action=review > > These are the primary issues I noticed. > > > LayoutTests/inspector/styles/styles-commit-editing.html:25 > > + treeElement.valueElement.textContent = "rgb(1"; > > doesn't the following sequence work? > treeElement.valueElement.textContent = "rgb("; > treeElement.valueElement.dispatchEvent(InspectorTest.createKeyEvent("U+0031")); > I need this to be processed synchronously, while key event handler only schedules a timer. > This could save us a fortune (and a ForTest method). > > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1644 > > + } else if (event.keyCode === WebInspector.KeyboardShortcut.Keys.Esc.code || event.keyIdentifier === "Escape") > > A better approach is to fix this to check a single event.keyIdentifier === "U+001B". This way we won't have to add two hacks. > Done. > > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1941 > > + return typeof this.originalPropertyText === "string"; > > If you don't use any other type than "string", why is the original condition changed? > I like searching for this.originalPropertyText occurrences more + it is compiler friendly. > > Source/WebCore/inspector/front-end/inspector.js:1555 > > + > > a new blank line might not be necessary Done. Committed r85324: <http://trac.webkit.org/changeset/85324> |