Fixing increment operation in LayoutUnit value.
Created attachment 226623 [details] Patch
Yeah, we don't have postfix increment operator on LayoutUnit. The fix looks good to me, but I'm not a reviewer.
Comment on attachment 226623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226623&action=review > Source/WebCore/ChangeLog:8 > + Fixing increment operation in LayoutUnit value. Although the change is straightforward, I suggest elaborating on the what is being fixed and why. Maybe something like: Use pre-incremeent operator for LayoutUnit instead of post-increment operator as LayoutUnit doesn't support the latter and we don't make use of the return value.
(In reply to comment #3) > (From update of attachment 226623 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226623&action=review > > > Source/WebCore/ChangeLog:8 > > + Fixing increment operation in LayoutUnit value. > > Although the change is straightforward, I suggest elaborating on the what is being fixed and why. Maybe something like: * "the what" => what > > Use pre-incremeent operator for LayoutUnit instead of post-increment operator as LayoutUnit doesn't support the latter and we don't make use of the return value. * pre-increment
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 226623 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=226623&action=review > > > > > Source/WebCore/ChangeLog:8 > > > + Fixing increment operation in LayoutUnit value. > > > > Although the change is straightforward, I suggest elaborating on the what is being fixed and why. Maybe something like: > > * "the what" => what > > > > > Use pre-incremeent operator for LayoutUnit instead of post-increment operator as LayoutUnit doesn't support the latter and we don't make use of the return value. > > * pre-increment OK, changing it...
Created attachment 226712 [details] Changes in ChangeLog
Comment on attachment 226712 [details] Changes in ChangeLog Rejecting attachment 226712 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 226712, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/6619990564274176
Created attachment 226713 [details] Forgot reviewed by
Comment on attachment 226713 [details] Forgot reviewed by Clearing flags on attachment: 226713 Committed r165617: <http://trac.webkit.org/changeset/165617>
Comment on attachment 226623 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226623&action=review > Source/WebCore/dom/Element.cpp:-650 > - value++; Was this failing to compile? If not, then we need to fix LayoutUnit so this fails to compile or compiles correctly. We can’t leave this as a trap for future programmers!
(In reply to comment #10) > (From update of attachment 226623 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226623&action=review > > > Source/WebCore/dom/Element.cpp:-650 > > - value++; > > Was this failing to compile? If not, then we need to fix LayoutUnit so this fails to compile or compiles correctly. We can’t leave this as a trap for future programmers! Darin, this was not compiling