Bug 130210 – Build fix with SUBPIXEL_LAYOUT disabled

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Thiago de Barros Lacerda]

Fixing increment operation in LayoutUnit value.

[Thiago de Barros Lacerda]

Created attachment 226623 [details]
Patch

[Zoltan Horvath]

Yeah, we don't have postfix increment operator on LayoutUnit.

The fix looks good to me, but I'm not a reviewer.

[Daniel Bates]

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.

[Daniel Bates]

(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

[Thiago de Barros Lacerda]

(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...

[Thiago de Barros Lacerda]

Created attachment 226712 [details]
Changes in ChangeLog

[WebKit Commit Bot]

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

[Thiago de Barros Lacerda]

Created attachment 226713 [details]
Forgot reviewed by

[WebKit Commit Bot]

Comment on attachment 226713 [details]
Forgot reviewed by

Clearing flags on attachment: 226713

Committed r165617: <http://trac.webkit.org/changeset/165617>

[Darin Adler]

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!

[Thiago de Barros Lacerda]

(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