[webkit-reviews] review granted: [Bug 20273] Allow incrementing/decrementing numbers in editable style properties with arrow keys : [Attachment 22638] Patch 2 of 2

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Timothy Hatcher
<timothy at hatcher.name>'s request for review:
Bug 20273: Allow incrementing/decrementing numbers in editable style properties
with arrow keys
https://bugs.webkit.org/show_bug.cgi?id=20273

Attachment 22638: Patch 2 of 2
https://bugs.webkit.org/attachment.cgi?id=22638&action=edit

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
+	 var wordRange =
selectionRange.startContainer.rangeOfWord(selectionRange.startOffset , "
\t\n\"':;,/()", this.listItemElement);

I think it would really improve readability to start putting these character
sets into named variables, like cssValueDelimeters or something.

+	     var changeAmount = (event.shiftKey ? 10 : ((event.altKey ||
numberNearZero) ? 0.1 : 1));

I think this would be a little clearer as if/elseif/else.

+	 if ((shorthandProperty || individualProperty) in foundProperties)

I think you should put "(shorthandProperty || individualProperty) into a local
variable since you keep repeating that construct.

Do you have any test cases to land with this?

r=me