Bug 85755

Summary:

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

Product:

WebKit

Reporter:

Lea Verou <lea>

Component:

CSS

Assignee:

Sumedha Widyadharma <sumedha.widyadharma>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

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

Priority:

Keywords:

WebExposed

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

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

Attachments:

Description	Flags
Proposed patch to add the 'ch' unit to webkit	none
Missing chromium expected results patch	none
Second attempt of a CSS3 ch unit patch with merged chromium results	none
3rd attempt of a CSS3 ch unit patch	none
CSS3 ch unit patch	none
Updated chromium layout test results	none
CSS3 ch unit patch	peter+ews: commit-queue-
CSS3 ch unit patch	none
CSS3 ch unit patch	webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-06	none
Archive of layout-test-results from gce-cr-linux-02	none
CSS3 ch unit patch	none
CSS3 ch unit patch	none
CSS3 ch unit patch	hyatt: review-, buildbot: commit-queue-
CSS3 ch unit patch	none
Text & pixel unit test	none
Text output	none
Pixel output	none
CSS3 ch unit patch (EWS only version)	none
CSS3 ch unit patch	none
CSS3 ch unit patch	none
CSS3 ch unit patch	none
CSS3 ch unit patch	none
CSS3 ch unit patch	hyatt: review+, webkit.review.bot: commit-queue-
CSS3 ch unit patch	none

Lea Verou

Reported 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)

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 attachment proposed patch, testcase, etc.

Bruno Abinader (history only)

Comment 1 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.

Sumedha Widyadharma

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

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

Bruno Abinader (history only)

Comment 3 2012-07-10 08:07:33 PDT

Created attachment 151463 [details] Missing chromium expected results patch

Sumedha Widyadharma

Comment 4 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.

Sumedha Widyadharma

Comment 5 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.

Sumedha Widyadharma

Comment 6 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.

Peter Beverloo

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

Bruno Abinader (history only)

Comment 8 2012-07-18 07:16:47 PDT

Created attachment 153006 [details] Updated chromium layout test results Updated chromium test results

Sumedha Widyadharma

Comment 9 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.

Bruno Abinader (history only)

Comment 10 2012-07-18 08:09:08 PDT

Adding W3 reference url.

Peter Beverloo (cr-android ews)

Comment 11 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

Sumedha Widyadharma

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

Created attachment 157997 [details] CSS3 ch unit patch Rebased for signature change of WebCore::formatNumber

WebKit Review Bot

Comment 13 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.

WebKit Review Bot

Comment 14 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.

Bruno Abinader (history only)

Comment 15 2012-08-13 07:53:16 PDT

Comment on attachment 157997 [details] CSS3 ch unit patch Resetting review and commit-queue flags, sorry folks :)

Sumedha Widyadharma

Comment 16 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.

WebKit Review Bot

Comment 17 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

WebKit Review Bot

Comment 18 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

WebKit Review Bot

Comment 19 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

WebKit Review Bot

Comment 20 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

Lamarque V. Souza

Comment 21 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

WebKit Review Bot

Comment 22 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.

Lamarque V. Souza

Comment 23 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.

WebKit Review Bot

Comment 24 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

Build Bot

Comment 25 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

Sumedha Widyadharma

Comment 26 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.

Lamarque V. Souza

Comment 27 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.

Lamarque V. Souza

Comment 28 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

WebKit Review Bot

Comment 29 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.

WebKit Review Bot

Comment 30 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

WebKit Review Bot

Comment 31 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

Build Bot

Comment 32 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

Build Bot

Comment 33 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

Build Bot

Comment 34 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

Tony Chang

Comment 35 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.

Tony Chang

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

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

Lamarque V. Souza

Comment 37 2013-01-21 07:56:22 PST

Created attachment 183785 [details] CSS3 ch unit patch Fix code style and add -expected.png file.

WebKit Review Bot

Comment 38 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.

Build Bot

Comment 39 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

Dave Hyatt

Comment 40 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.

Lamarque V. Souza

Comment 41 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.

WebKit Review Bot

Comment 42 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.

Lamarque V. Souza

Comment 43 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.

Lamarque V. Souza

Comment 44 2013-01-22 18:49:49 PST

Created attachment 184102 [details] Text output

Lamarque V. Souza

Comment 45 2013-01-22 18:53:18 PST

Created attachment 184103 [details] Pixel output

WebKit Review Bot

Comment 46 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.

Lamarque V. Souza

Comment 47 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

Lamarque V. Souza

Comment 48 2013-01-23 08:28:22 PST

Created attachment 184243 [details] CSS3 ch unit patch Change unit test to text-only.

WebKit Review Bot

Comment 49 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.

WebKit Review Bot

Comment 50 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.

Tony Chang

Comment 51 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().

Lamarque V. Souza

Comment 52 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().

Tony Chang

Comment 53 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.

Build Bot

Comment 54 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

Lamarque V. Souza

Comment 55 2013-01-24 08:44:19 PST

Created attachment 184510 [details] CSS3 ch unit patch Fixes to unit tests.

WebKit Review Bot

Comment 56 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.

Tony Chang

Comment 57 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.

Lamarque V. Souza

Comment 58 2013-01-24 17:13:01 PST

Created attachment 184616 [details] CSS3 ch unit patch Small fixes to unit tests.

WebKit Review Bot

Comment 59 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.

Dave Hyatt

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

Comment on attachment 184616 [details] CSS3 ch unit patch r=me

WebKit Review Bot

Comment 61 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

Lamarque V. Souza

Comment 62 2013-02-07 14:53:56 PST

Created attachment 187182 [details] CSS3 ch unit patch Patch for landing.

WebKit Review Bot

Comment 63 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.

WebKit Review Bot

Comment 64 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

Build Bot

Comment 65 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

Lamarque V. Souza

Comment 66 2013-02-08 15:50:29 PST

Created attachment 187376 [details] CSS3 ch unit patch Patch for landing.

WebKit Review Bot

Comment 67 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.

Build Bot

Comment 68 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

Dave Hyatt

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

Comment on attachment 187376 [details] CSS3 ch unit patch r=me

WebKit Review Bot

Comment 70 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

Lamarque V. Souza

Comment 71 2013-02-14 12:22:37 PST

Created attachment 188399 [details] CSS3 ch unit patch Patch rebased on top of current git revision.

WebKit Review Bot

Comment 72 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>

Bruno Abinader (history only)

Comment 73 2013-04-29 06:50:55 PDT

All reviewed patches have been landed. Closing bug.

Gérard Talbot (no longer involved)

Comment 74 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.

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