[webkit-reviews] review granted: [Bug 40522] Web Inspector: up/down keys are not treating hex numbers properly while editing styles. : [Attachment 87845] [PATCH] Suggested solution

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 1 05:54:49 PDT 2011


Yury Semikhatsky <yurys at chromium.org> has granted Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 40522: Web Inspector: up/down keys are not treating hex numbers properly
while editing styles.
https://bugs.webkit.org/show_bug.cgi?id=40522

Attachment 87845: [PATCH] Suggested solution
https://bugs.webkit.org/attachment.cgi?id=87845&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=87845&action=review

> LayoutTests/inspector/styles/up-down-numerics-and-colors.html:6
> +    color: #FF2;

Can you use #FAF or something that will overflow on increment?

> LayoutTests/inspector/styles/up-down-numerics-and-colors.html:25
> +	       section =
WebInspector.panels.elements.sidebarPanes.styles.sections[0][2];

Missing "var" before section declaration. Also could we get the section in a
more reliable way?

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1650
> +	   matches = /(.*?#)([\da-fA-F]+)(.*)/.exec(wordString);

.*?# - > .*#

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1658
> +	       matches =
/(.*?)(-?(?:\d+(?:\.\d+)?|\.\d+))(.*)/.exec(wordString);

.*? - > .*

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1740
> +	   var arrowKeyPressed = (event.keyIdentifier === "Up" ||
event.keyIdentifier === "Down");

These shortcuts look overcomplicated too me, could we use a simpler scheme?


More information about the webkit-reviews mailing list