Bug 130210 - Build fix with SUBPIXEL_LAYOUT disabled
Summary: Build fix with SUBPIXEL_LAYOUT disabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thiago de Barros Lacerda
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-13 14:28 PDT by Thiago de Barros Lacerda
Modified: 2014-03-17 07:32 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.24 KB, patch)
2014-03-13 14:30 PDT, Thiago de Barros Lacerda
dbates: review+
thiago.lacerda: commit-queue-
Details | Formatted Diff | Diff
Changes in ChangeLog (1.39 KB, patch)
2014-03-14 07:41 PDT, Thiago de Barros Lacerda
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Forgot reviewed by (1.39 KB, patch)
2014-03-14 07:47 PDT, Thiago de Barros Lacerda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago de Barros Lacerda 2014-03-13 14:28:15 PDT
Fixing increment operation in LayoutUnit value.
Comment 1 Thiago de Barros Lacerda 2014-03-13 14:30:32 PDT
Created attachment 226623 [details]
Patch
Comment 2 Zoltan Horvath 2014-03-13 21:35:34 PDT
Yeah, we don't have postfix increment operator on LayoutUnit.

The fix looks good to me, but I'm not a reviewer.
Comment 3 Daniel Bates 2014-03-13 23:46:53 PDT
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.
Comment 4 Daniel Bates 2014-03-13 23:48:03 PDT
(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
Comment 5 Thiago de Barros Lacerda 2014-03-14 07:39:10 PDT
(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...
Comment 6 Thiago de Barros Lacerda 2014-03-14 07:41:51 PDT
Created attachment 226712 [details]
Changes in ChangeLog
Comment 7 WebKit Commit Bot 2014-03-14 07:44:04 PDT
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
Comment 8 Thiago de Barros Lacerda 2014-03-14 07:47:36 PDT
Created attachment 226713 [details]
Forgot reviewed by
Comment 9 WebKit Commit Bot 2014-03-14 08:24:37 PDT
Comment on attachment 226713 [details]
Forgot reviewed by

Clearing flags on attachment: 226713

Committed r165617: <http://trac.webkit.org/changeset/165617>
Comment 10 Darin Adler 2014-03-15 14:22:12 PDT
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!
Comment 11 Thiago de Barros Lacerda 2014-03-17 07:32:35 PDT
(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