85755 – [CSS3 Values] Add support for the "ch" unit

Bug 85755 - [CSS3 Values] Add support for the "ch" unit

Summary: [CSS3 Values] Add support for the "ch" unit

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	CSS (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Sumedha Widyadharma

URL:	http://www.w3.org/TR/css3-values/#ch-...
Keywords:	WebExposed

Depends on:
Blocks:

Reported:	2012-05-06 17:49 PDT by Lea Verou
Modified:	2019-03-25 08:28 PDT (History)
CC List:	31 users (show)

See Also:

Attachments
Proposed patch to add the 'ch' unit to webkit (17.01 KB, patch) 2012-07-06 01:54 PDT, Sumedha Widyadharma	no flags	Details \| Formatted Diff \| Diff
Missing chromium expected results patch (6.70 KB, patch) 2012-07-10 08:07 PDT, Bruno Abinader (history only)	no flags	Details \| Formatted Diff \| Diff
Second attempt of a CSS3 ch unit patch with merged chromium results (25.63 KB, patch) 2012-07-11 00:06 PDT, Sumedha Widyadharma	no flags	Details \| Formatted Diff \| Diff
3rd attempt of a CSS3 ch unit patch (25.01 KB, patch) 2012-07-11 00:40 PDT, Sumedha Widyadharma	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (20.64 KB, patch) 2012-07-11 00:58 PDT, Sumedha Widyadharma	no flags	Details \| Formatted Diff \| Diff
Updated chromium layout test results (53.46 KB, patch) 2012-07-18 07:16 PDT, Bruno Abinader (history only)	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (39.01 KB, patch) 2012-07-18 07:38 PDT, Sumedha Widyadharma	peter+ews: commit-queue-	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (39.04 KB, patch) 2012-08-13 07:45 PDT, Sumedha Widyadharma	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (39.04 KB, patch) 2012-08-13 07:57 PDT, Sumedha Widyadharma	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
Archive of layout-test-results from gce-cr-linux-06 (357.17 KB, application/zip) 2012-08-13 10:52 PDT, WebKit Review Bot	no flags	Details
Archive of layout-test-results from gce-cr-linux-02 (404.37 KB, application/zip) 2012-08-13 12:15 PDT, WebKit Review Bot	no flags	Details
CSS3 ch unit patch (37.64 KB, patch) 2012-11-15 11:27 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (37.79 KB, patch) 2012-12-17 15:37 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (84.94 KB, patch) 2013-01-21 07:56 PST, Lamarque V. Souza	hyatt: review- buildbot: commit-queue-	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (83.81 KB, patch) 2013-01-21 14:30 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
Text & pixel unit test (8.34 KB, text/html) 2013-01-22 18:48 PST, Lamarque V. Souza	no flags	Details
Text output (2.62 KB, text/plain) 2013-01-22 18:49 PST, Lamarque V. Souza	no flags	Details
Pixel output (78.44 KB, image/png) 2013-01-22 18:53 PST, Lamarque V. Souza	no flags	Details
CSS3 ch unit patch (EWS only version) (43.99 KB, patch) 2013-01-23 06:17 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (28.66 KB, patch) 2013-01-23 08:28 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (28.99 KB, patch) 2013-01-24 08:44 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (29.01 KB, patch) 2013-01-24 17:13 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (28.65 KB, patch) 2013-02-07 14:53 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (28.65 KB, patch) 2013-02-08 15:50 PST, Lamarque V. Souza	hyatt: review+ webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
CSS3 ch unit patch (28.69 KB, patch) 2013-02-14 12:22 PST, Lamarque V. Souza	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (20) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Lea Verou 2012-05-06 17:49:05 PDT

Draft: http://www.w3.org/TR/css3-values/#ch-unit

Gecko and Trident already support this. It’s immensely useful when dealing with monospace fonts.

(I searched a lot; Couldn’t find an existing report. Sorry if it’s a dupe)

Comment 1 Bruno Abinader (history only) 2012-06-15 08:23:34 PDT

Sumedha has started working on this issue and already has a working patch that enables the "ch" CSS3 value feature, we are now in the process of finishing a proper layout test for this.

Comment 2 Sumedha Widyadharma 2012-07-06 01:54:34 PDT

Created attachment 151038 [details]
Proposed patch to add the 'ch' unit to webkit

Comment 3 Bruno Abinader (history only) 2012-07-10 08:07:33 PDT

Created attachment 151463 [details]
Missing chromium expected results patch

Comment 4 Sumedha Widyadharma 2012-07-11 00:06:29 PDT

Created attachment 151616 [details]
Second attempt of a CSS3 ch unit patch with merged chromium results

I merged Brunos patch into mine and put his name in the ChangeLog entry. I hope that's ok.

Comment 5 Sumedha Widyadharma 2012-07-11 00:40:35 PDT

Created attachment 151623 [details]
3rd attempt of a CSS3 ch unit patch

I removed a spurious new line from the previous patch, no functional changes.

Comment 6 Sumedha Widyadharma 2012-07-11 00:58:19 PDT

Created attachment 151630 [details]
CSS3 ch unit patch

I removed the failing chromium pixel test. The layout test should suffice.

Comment 7 Peter Beverloo 2012-07-16 07:02:47 PDT

Comment on attachment 151630 [details]
CSS3 ch unit patch

View in context: https://bugs.webkit.org/attachment.cgi?id=151630&action=review

Some informal nits.. thanks!

> LayoutTests/ChangeLog:3
> +        Added tests for the CSS3 'ch' unit and expected results for

Please use the common format for ChangeLogs:
<bug title>
<bug url>

<description>

> LayoutTests/ChangeLog:10
> +        * fast/css/css3-ch-unit.html: Added.

You have one test right now, only testing a monospace font. Other things we'll want to test here include normal (non-monospace) fonts, font fallback behavior (i.e. if the first font in the font-family list isn't available) and behavior when it's used in "font-size", in which case they should refer to the computed font metrics of the parent element (see 5.1.1 in the spec). How will this behave in getComputedStyle()? Maybe it's a good idea as well to include a test for verifying that this works in pseudo-elements such as ::first-line.

> LayoutTests/fast/css/css3-ch-unit.html:1
> +<html>

nit: should use standards mode. <!doctype html>

> LayoutTests/fast/css/css3-ch-unit.html:6
> +		font-height: 12px;

font-size? :)

> LayoutTests/fast/css/css3-ch-unit.html:12
> +		width: 4ch

nit: semi-colons after the properties.

Comment 8 Bruno Abinader (history only) 2012-07-18 07:16:47 PDT

Created attachment 153006 [details]
Updated chromium layout test results

Updated chromium test results

Comment 9 Sumedha Widyadharma 2012-07-18 07:38:05 PDT

Created attachment 153012 [details]
CSS3 ch unit patch 

Updated as per comments. If anyone can think of other good tests, maybe involving getComputedStyle, please comment. I couldn't think of something good.

Comment 10 Bruno Abinader (history only) 2012-07-18 08:09:08 PDT

Adding W3 reference url.

Comment 11 Peter Beverloo (cr-android ews) 2012-08-10 10:23:00 PDT

Comment on attachment 153012 [details]
CSS3 ch unit patch 

Attachment 153012 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13470645

Comment 12 Sumedha Widyadharma 2012-08-13 07:45:25 PDT

Created attachment 157997 [details]
CSS3 ch unit patch

Rebased for signature change of WebCore::formatNumber

Comment 13 WebKit Review Bot 2012-08-13 07:45:29 PDT

Comment on attachment 157997 [details]
CSS3 ch unit patch

Rejecting attachment 157997 [details] from review queue.

sumedha.widyadharma@basyskom.com does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.

Comment 14 WebKit Review Bot 2012-08-13 07:46:11 PDT

Comment on attachment 157997 [details]
CSS3 ch unit patch

Rejecting attachment 157997 [details] from commit-queue.

sumedha.widyadharma@basyskom.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

Comment 15 Bruno Abinader (history only) 2012-08-13 07:53:16 PDT

Comment on attachment 157997 [details]
CSS3 ch unit patch

Resetting review and commit-queue flags, sorry folks :)

Comment 16 Sumedha Widyadharma 2012-08-13 07:57:10 PDT

Created attachment 158001 [details]
CSS3 ch unit patch

Same patch as before, I had a brainflake when setting the flags.

Comment 17 WebKit Review Bot 2012-08-13 10:52:33 PDT

Comment on attachment 158001 [details]
CSS3 ch unit patch

Attachment 158001 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13480828

New failing tests:
fast/css/css3-ch-unit.html

Comment 18 WebKit Review Bot 2012-08-13 10:52:38 PDT

Created attachment 158042 [details]
Archive of layout-test-results from gce-cr-linux-06

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-06  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Comment 19 WebKit Review Bot 2012-08-13 12:15:15 PDT

Comment on attachment 158001 [details]
CSS3 ch unit patch

Attachment 158001 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13493211

New failing tests:
fast/frames/cached-frame-counter.html
fast/css/css3-ch-unit.html

Comment 20 WebKit Review Bot 2012-08-13 12:15:22 PDT

Created attachment 158067 [details]
Archive of layout-test-results from gce-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Comment 21 Lamarque V. Souza 2012-11-15 11:27:49 PST

Created attachment 174491 [details]
CSS3 ch unit patch

Fixes to make patch apply and compile with current git revision

Comment 22 WebKit Review Bot 2012-11-15 11:30:58 PST

Attachment 174491 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/css/CSSParser.cpp:1602:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSParser.cpp:1607:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:166:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:167:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:168:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 23 Lamarque V. Souza 2012-11-15 11:38:51 PST

(In reply to comment #22)
> Attachment 174491 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
> Source/WebCore/css/CSSParser.cpp:1602:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Source/WebCore/css/CSSParser.cpp:1607:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Source/WebCore/css/CSSPrimitiveValue.h:166:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Source/WebCore/css/CSSPrimitiveValue.h:167:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Source/WebCore/css/CSSPrimitiveValue.h:168:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
> Total errors found: 5 in 15 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Hmmm the style used is equal to what is already used in those two files. Does anyone know what is supposed to be the correct style in this case? Also can someone obsolete https://bugs.webkit.org/attachment.cgi?id=158001 ? I do not have permission to do so.

Comment 24 WebKit Review Bot 2012-11-15 12:34:45 PST

Comment on attachment 174491 [details]
CSS3 ch unit patch

Attachment 174491 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14838804

New failing tests:
fast/css/css3-ch-unit.html

Comment 25 Build Bot 2012-11-16 05:27:59 PST

Comment on attachment 174491 [details]
CSS3 ch unit patch

Attachment 174491 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14848774

New failing tests:
fast/css/css3-ch-unit.html

Comment 26 Sumedha Widyadharma 2012-11-16 05:33:33 PST

[...]
> Hmmm the style used is equal to what is already used in those two files. Does anyone know what is supposed to be the correct style in this case? Also can someone obsolete https://bugs.webkit.org/attachment.cgi?id=158001 ? I do not have permission to do so.

I don't have an answer to your question, but I obsoleted my patch.

Comment 27 Lamarque V. Souza 2012-11-16 06:29:57 PST

Hi, Sumedha Widyadharma. Can you take a look at LayoutTests/platform/qt/fast/css/css3-ch-unit-expected.txt and LayoutTests/platform/chromium-linux/fast/css/css3-ch-unit-expected.txt and check if they are correct? You have more knowledge about ch-unit than me.

Comment 28 Lamarque V. Souza 2012-12-17 15:37:25 PST

Created attachment 179815 [details]
CSS3 ch unit patch

Fixes to make patch apply and compile with current git revision

Comment 29 WebKit Review Bot 2012-12-17 15:40:51 PST

Attachment 179815 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/css/CSSParser.cpp:1632:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSParser.cpp:1637:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:166:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:167:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/css/CSSPrimitiveValue.h:168:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 13 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 30 WebKit Review Bot 2012-12-17 23:12:44 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

Attachment 179815 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15407037

New failing tests:
fast/css/css3-ch-unit.html

Comment 31 WebKit Review Bot 2012-12-18 00:13:52 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

Attachment 179815 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15411016

New failing tests:
fast/css/css3-ch-unit.html

Comment 32 Build Bot 2012-12-18 05:00:53 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

Attachment 179815 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15409113

New failing tests:
fast/css/css3-ch-unit.html

Comment 33 Build Bot 2012-12-18 06:17:02 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

Attachment 179815 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15418087

New failing tests:
fast/css/css3-ch-unit.html

Comment 34 Build Bot 2013-01-16 23:36:18 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

Attachment 179815 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15914696

New failing tests:
fast/css/css3-ch-unit.html

Comment 35 Tony Chang 2013-01-17 10:22:34 PST

Comment on attachment 179815 [details]
CSS3 ch unit patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179815&action=review

>> Source/WebCore/css/CSSParser.cpp:1632
>> +           || (value->unit >= CSSPrimitiveValue::CSS_TURN && value->unit <= CSSPrimitiveValue::CSS_CHS)
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

I would just fix these and the lines in the same expression.  You should indent 4 spaces in from the ASSERT (8 spaces from the left edge of the file).

>> Source/WebCore/css/CSSParser.cpp:1637
>> +           || (value->unit >= CSSPrimitiveValue::CSS_TURN && value->unit <= CSSPrimitiveValue::CSS_CHS)
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

I would fix this too.

>> Source/WebCore/css/CSSPrimitiveValue.h:118
>> +        CSS_CHS = 109,
> 
> enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]

It's OK to keep following the existing style here.  It would be nice for someone to fix this in a separate patch.

>> Source/WebCore/css/CSSPrimitiveValue.h:166
>> +               || m_primitiveUnitType == CSS_EXS
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

These are lines you added, so they should follow the style guide and be indented 4 spaces from the return.

> LayoutTests/ChangeLog:10
> +        * fast/css/css3-ch-unit.html: Added.
> +        * platform/chromium-linux/fast/css/css3-ch-unit-expected.txt: Added.
> +        * platform/qt/fast/css/css3-ch-unit-expected.txt: Added.

This test is failing on the chromium EWS bots because it doesn't have the pixel result (png file checked in).  Maybe we can do better and make this a dump as text test.  For example, you could have a div with the width of 1ch and you could have a div that shrink wraps (e.g., inline-block) the number 0.  The divs should be the same width which you could check in javascript.  For testing font-size:1ch, you might be able to use the Ahem font and get the same number on all platforms.  You might be able to do the pseudo-element tests using a ref test.

If you find that something can only be tested using a pixel test, we should at least split that off into as small and focused of a test as possible to make it easier for people maintaining the tests to tell if they are correct or not.

Comment 36 Tony Chang 2013-01-17 10:24:56 PST

Mitz or Hyatt would be good reviewers for the changes to SimpleFontData.

Comment 37 Lamarque V. Souza 2013-01-21 07:56:22 PST

Created attachment 183785 [details]
CSS3 ch unit patch

Fix code style and add -expected.png file.

Comment 38 WebKit Review Bot 2013-01-21 07:59:31 PST

Attachment 183785 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/chromium-linux/fast/css/css3-ch-unit-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 9 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 39 Build Bot 2013-01-21 08:55:19 PST

Comment on attachment 183785 [details]
CSS3 ch unit patch

Attachment 183785 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16038089

New failing tests:
fast/css/css3-ch-unit.html

Comment 40 Dave Hyatt 2013-01-21 10:45:06 PST

Comment on attachment 183785 [details]
CSS3 ch unit patch

I would prefer that the zero width get put into the font metrics (similar to what we do with other size information like xheight). That way computeLengthDouble is consistent across all unit types in that it talks to the font metrics for this sort of information.

Comment 41 Lamarque V. Souza 2013-01-21 14:30:16 PST

Created attachment 183832 [details]
CSS3 ch unit patch

Move zeroWidth() to FontMetrics and add css3-ch-unit.html to mac-wk2's TestExpectations file.

Comment 42 WebKit Review Bot 2013-01-21 14:32:50 PST

Attachment 183832 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
LayoutTests/platform/chromium-linux/fast/css/css3-ch-unit-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 9 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 43 Lamarque V. Souza 2013-01-22 18:48:39 PST

Created attachment 184100 [details]
Text & pixel unit test

This is a unit test that works for both text and pixel, I would like to read comments about it. I will attach the -expected.{txt,png} later.

Comment 44 Lamarque V. Souza 2013-01-22 18:49:49 PST

Created attachment 184102 [details]
Text output

Comment 45 Lamarque V. Souza 2013-01-22 18:53:18 PST

Created attachment 184103 [details]
Pixel output

Comment 46 WebKit Review Bot 2013-01-22 18:53:30 PST

Attachment 184100 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 47 Lamarque V. Souza 2013-01-23 06:17:24 PST

Created attachment 184214 [details]
CSS3 ch unit patch (EWS only version)

EWS only version, see https://bugs.webkit.org/show_bug.cgi?id=106819

Comment 48 Lamarque V. Souza 2013-01-23 08:28:22 PST

Created attachment 184243 [details]
CSS3 ch unit patch

Change unit test to text-only.

Comment 49 WebKit Review Bot 2013-01-23 10:11:58 PST

Attachment 184243 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 50 WebKit Review Bot 2013-01-23 10:33:05 PST

Attachment 184214 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/mac-wk2/TestExpectations', u'Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig', u'Source/WebCore/ChangeLog', u'Source/WebCore/Configurations/FeatureDefines.xcconfig', u'Source/WebCore/GNUmakefile.features.am.in', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/css/CSSPropertyNames.in', u'Source/WebCore/css/CSSRule.idl', u'Source/WebCore/css/CSSValueKeywords.in', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp', u'Source/WebKit/chromium/features.gypi', u'Source/WebKit/mac/Configurations/FeatureDefines.xcconfig', u'Source/WebKit2/Configurations/FeatureDefines.xcconfig', u'Source/cmake/OptionsCommon.cmake', u'Source/cmake/WebKitFeatures.cmake', u'Tools/Scripts/webkitperl/FeatureList.pm', u'Tools/qmake/mkspecs/features/features.pri', u'WebKitLibraries/win/tools/vsprops/FeatureDefines.vsprops', u'WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.vsprops', u'configure.ac']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 51 Tony Chang 2013-01-23 10:43:04 PST

Comment on attachment 184243 [details]
CSS3 ch unit patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184243&action=review

> LayoutTests/fast/css/css3-ch-unit-expected.txt:28
> +Only 'PASS' should be visible in a web browser (it is Ok to see some 'FAIL' in DumpRenderTree):
> +PASSFAIL

This seems bad.  If the test is passing, we shouldn't write "FAIL" to the output.  If the test output changes, it makes it very hard for someone to tell if the change is OK or not if they haven't worked in this area.

Sometimes at the end of a test, we set display:none on the test contents to hide this extra test.  You could make it only do that if running in the test harness (check for window.testRunner).

> LayoutTests/fast/css/css3-ch-unit.html:163
> +				pass();
> +				testPassed("forcedsmall is chsmall");

Why do you need both pass() and testPassed()?  Same question about fail() and testFailed().

Comment 52 Lamarque V. Souza 2013-01-23 10:54:33 PST

(In reply to comment #51)
> (From update of attachment 184243 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184243&action=review
> 
> > LayoutTests/fast/css/css3-ch-unit-expected.txt:28
> > +Only 'PASS' should be visible in a web browser (it is Ok to see some 'FAIL' in DumpRenderTree):
> > +PASSFAIL
> 
> This seems bad.  If the test is passing, we shouldn't write "FAIL" to the output.  If the test output changes, it makes it very hard for someone to tell if the change is OK or not if they haven't worked in this area.

We can still when the test in a browser, in that case the word "FAIL" should not appear. That is how the pixel test used to run and I keep it that way. I can change the word "FAIL" to something else like "SHOULD_NOT_APPEAR".
 
> Sometimes at the end of a test, we set display:none on the test contents to hide this extra test.  You could make it only do that if running in the test harness (check for window.testRunner).

I will try that.
 
> > LayoutTests/fast/css/css3-ch-unit.html:163
> > +				pass();
> > +				testPassed("forcedsmall is chsmall");
> 
> Why do you need both pass() and testPassed()?  Same question about fail() and testFailed().

testPassed()/testFailed() do not print anything when opening css3-ch-unit.html in a web browser. If display:none works then I can remove pass()/fail().

Comment 53 Tony Chang 2013-01-23 11:00:31 PST

Comment on attachment 184243 [details]
CSS3 ch unit patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184243&action=review

As a minor nit, we try to use similar style as C++ in our tests.  Mainly, that means 4 space indents (no tabs).

>>> LayoutTests/fast/css/css3-ch-unit.html:163
>>> +				testPassed("forcedsmall is chsmall");
>> 
>> Why do you need both pass() and testPassed()?  Same question about fail() and testFailed().
> 
> testPassed()/testFailed() do not print anything when opening css3-ch-unit.html in a web browser. If display:none works then I can remove pass()/fail().

I see, I would replace this with shouldBeFalse calls.  E.g., "shouldBeFalse(w1 !== w2 || h1 !== h2)".

> LayoutTests/fast/css/css3-ch-unit.html:179
> +		if((ps1h > ps2h) && (ps1h > ps3h)) {
> +			pass();
> +			testPassed('The first line is higher than the other two.');
> +		} else {

You could use shouldBeTrue here.

Comment 54 Build Bot 2013-01-23 12:26:22 PST

Comment on attachment 184214 [details]
CSS3 ch unit patch (EWS only version)

Attachment 184214 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16074328

Comment 55 Lamarque V. Souza 2013-01-24 08:44:19 PST

Created attachment 184510 [details]
CSS3 ch unit patch

Fixes to unit tests.

Comment 56 WebKit Review Bot 2013-01-24 14:30:18 PST

Attachment 184510 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 57 Tony Chang 2013-01-24 15:51:28 PST

Comment on attachment 184510 [details]
CSS3 ch unit patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184510&action=review

The test looks OK to me.  Hyatt or someone else with more knowledge of fonts should review the rest of the code.

> LayoutTests/fast/css/css3-ch-unit.html:50
> +<script type="text/javascript">
> +//testRunner.dumpAsText(true);
> +</script>

Please remove this dead code.

> LayoutTests/fast/css/css3-ch-unit.html:180
> +        // Text with '<span class="fail">' must not appear in DumpRenderTree's output, only when opening this file in a web browser.
> +        if (window.testRunner) {
> +            var CSSRules = 'rules';
> +            for (var i = 0; i < document.styleSheets[0][CSSRules].length; i++)
> +                if (document.styleSheets[0][CSSRules][i].selectorText == ".fail")
> +                    document.styleSheets[0][CSSRules][i].style['display'] = 'none';
> +    }

Wrong indent on the closing brace.
This is fine, I would have just put all the tests into a div and display: none the whole thing.  Then you don't have any test spew in the -expected.txt file.

Comment 58 Lamarque V. Souza 2013-01-24 17:13:01 PST

Created attachment 184616 [details]
CSS3 ch unit patch

Small fixes to unit tests.

Comment 59 WebKit Review Bot 2013-01-24 22:34:44 PST

Attachment 184616 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:118:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:121:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:124:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:127:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:129:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:134:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 60 Dave Hyatt 2013-02-07 12:36:24 PST

Comment on attachment 184616 [details]
CSS3 ch unit patch

r=me

Comment 61 WebKit Review Bot 2013-02-07 12:42:18 PST

Comment on attachment 184616 [details]
CSS3 ch unit patch

Rejecting attachment 184616 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 184616, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
mpleFontData.cpp
patching file Source/WebCore/platform/graphics/SimpleFontData.h
patching file Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/css/css3-ch-unit-expected.txt
patching file LayoutTests/fast/css/css3-ch-unit.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'David Hyatt']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16408434

Comment 62 Lamarque V. Souza 2013-02-07 14:53:56 PST

Created attachment 187182 [details]
CSS3 ch unit patch

Patch for landing.

Comment 63 WebKit Review Bot 2013-02-07 14:59:48 PST

Attachment 187182 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:119:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:122:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:125:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:128:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:132:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:135:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 64 WebKit Review Bot 2013-02-07 15:48:44 PST

Comment on attachment 187182 [details]
CSS3 ch unit patch

Rejecting attachment 187182 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'validate-changelog', '--non-interactive', 187182, '--port=chromium-xvfb']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Dave Hyatt found in /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py.
/mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/16445393

Comment 65 Build Bot 2013-02-07 21:02:32 PST

Comment on attachment 187182 [details]
CSS3 ch unit patch

Attachment 187182 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16445500

New failing tests:
http/tests/cache/cached-main-resource.html

Comment 66 Lamarque V. Souza 2013-02-08 15:50:29 PST

Created attachment 187376 [details]
CSS3 ch unit patch

Patch for landing.

Comment 67 WebKit Review Bot 2013-02-08 15:55:29 PST

Attachment 187376 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/css/css3-ch-unit-expected.txt', u'LayoutTests/fast/css/css3-ch-unit.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/css/CSSCalculationValue.cpp', u'Source/WebCore/css/CSSGrammar.y.in', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParserValues.cpp', u'Source/WebCore/css/CSSPrimitiveValue.cpp', u'Source/WebCore/css/CSSPrimitiveValue.h', u'Source/WebCore/platform/graphics/FontMetrics.h', u'Source/WebCore/platform/graphics/SimpleFontData.cpp', u'Source/WebCore/platform/graphics/SimpleFontData.h', u'Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp']" exit_code: 1
Source/WebCore/css/CSSPrimitiveValue.h:119:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:122:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:125:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:128:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:130:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:131:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:132:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/css/CSSPrimitiveValue.h:135:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 8 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 68 Build Bot 2013-02-09 02:04:25 PST

Comment on attachment 187376 [details]
CSS3 ch unit patch

Attachment 187376 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16465321

New failing tests:
compositing/checkerboard.html
accessibility/anchor-linked-anonymous-block-crash.html
compositing/absolute-inside-out-of-view-fixed.html
animations/3d/matrix-transform-type-animation.html
http/tests/cache/cancel-multiple-post-xhrs.html
animations/3d/state-at-end-event-transform.html
animations/animation-add-events-in-handler.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/cache/history-only-cached-subresource-loads.html
compositing/bounds-in-flipped-writing-mode.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
accessibility/accessibility-object-detached.html
accessibility/anonymous-render-block-in-continuation-causes-crash.html
animations/animation-controller-drt-api.html
compositing/absolute-position-changed-with-composited-parent-layer.html
compositing/absolute-position-changed-in-composited-layer.html
http/tests/cache/iframe-304-crash.html
animations/3d/transform-perspective.html
http/tests/cache/cancel-during-failure-crash.html
canvas/philip/tests/2d.canvas.reference.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html
canvas/philip/tests/2d.clearRect.basic.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
http/tests/cache/cached-main-resource.html
canvas/philip/tests/2d.clearRect+fillRect.basic.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html

Comment 69 Dave Hyatt 2013-02-14 11:27:01 PST

Comment on attachment 187376 [details]
CSS3 ch unit patch

r=me

Comment 70 WebKit Review Bot 2013-02-14 11:52:29 PST

Comment on attachment 187376 [details]
CSS3 ch unit patch

Rejecting attachment 187376 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 187376, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
mpleFontData.cpp
patching file Source/WebCore/platform/graphics/SimpleFontData.h
patching file Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/css/css3-ch-unit-expected.txt
patching file LayoutTests/fast/css/css3-ch-unit.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'David Hyatt']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16581451

Comment 71 Lamarque V. Souza 2013-02-14 12:22:37 PST

Created attachment 188399 [details]
CSS3 ch unit patch

Patch rebased on top of current git revision.

Comment 72 WebKit Review Bot 2013-02-14 13:20:29 PST

Comment on attachment 188399 [details]
CSS3 ch unit patch

Clearing flags on attachment: 188399

Committed r142904: <http://trac.webkit.org/changeset/142904>

Comment 73 Bruno Abinader (history only) 2013-04-29 06:50:55 PDT

All reviewed patches have been landed. Closing bug.

Comment 74 Gérard Talbot 2019-03-25 08:28:15 PDT

(In reply to Sumedha Widyadharma from comment #9)
> If anyone can think of other good tests (...)

These 6 (vertical writing-mode and text-orientation) tests

http://w3c-test.org/css/css-values/ch-unit-002.html

http://w3c-test.org/css/css-values/ch-unit-003.html

http://w3c-test.org/css/css-values/ch-unit-004.html

http://w3c-test.org/css/css-values/ch-unit-010.html

http://w3c-test.org/css/css-values/ch-unit-011.html

http://w3c-test.org/css/css-values/ch-unit-012.html

fail in latest stable Safari 12 and in latest Safari Technology Preview. The weird thing is that these 6 tests pass in Epiphany Technological Preview 3.31.90-8061c1929 (WebKitGTK 2.24.0). 

I will file a bug report about this in a few min.

abarth
abecsi
browserbugs2
bruno.abinader
buildbot
cmarcelo
dbates
dglazkov
gyuyoung.kim
hyatt
igor.oliveira
joethomas
kling
koivisto
lamarque
macpherson
menard
noam
ojan.autocc
peter
peter+ews
phiw2
rakuco
rniwa
simon.fraser
sumedha.widyadharma
syoichi
tony
vestbo
webkit.review.bot
zan