Bug 68040

Created attachment 107252 [details] Patch

Attachment 107252 [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/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 107252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107252&action=review > Source/WebCore/css/CSSParser.cpp:5188 > + m_allowImage = m_allowSlash = m_requireWidth = m_requireOutset = false; Normally we don’t do those all on one line. Can you put them on their own lines?

Comment on attachment 107252 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107252&action=review > LayoutTests/ChangeLog:7 > + * fast/borders/border-image-scrambled.html: Added. Could we test this with computed style instead, so we get a platform-independent test?

You should address Darin's comments, and the style bot's request for a bug number in the change log.

(In reply to comment #3) > (From update of attachment 107252 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=107252&action=review > > > Source/WebCore/css/CSSParser.cpp:5188 > > + m_allowImage = m_allowSlash = m_requireWidth = m_requireOutset = false; > > Normally we don’t do those all on one line. Can you put them on their own lines? (In reply to comment #4) > (From update of attachment 107252 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=107252&action=review > > > LayoutTests/ChangeLog:7 > > + * fast/borders/border-image-scrambled.html: Added. > > Could we test this with computed style instead, so we get a platform-independent test? I explained this on IRC, but basically the reason I didn't do this is we are suffering badly from a dearth of shadow, border-image, and background pixel results. For these particular features, pixels are king, and we need to have more (not fewer) samples. Therefore when I write new tests in these areas, I deliberately bias towards pixel results, since they give us more chances to catch mistakes made in the rendering itself.

Fixed in r95058.

Reopen, because it was rolled out by http://trac.webkit.org/changeset/95077 See https://bugs.webkit.org/show_bug.cgi?id=68051 for details.

Created attachment 107351 [details] Patch This patch is identical to the previous patch but it ensures that when you completely fail to parse any sub-property that you bail instead of infinite looping trying to examine the same property over and over again.

Attachment 107351 [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/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 107352 [details] Patch that passes style queue.

Attachment 107352 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2 Last 3072 characters of output: p://src.chromium.org/svn/trunk/src/build@100742 should_process: True requirements: set(['./']) name: net url: http://src.chromium.org/svn/trunk/src/net@100742 parsed_url: http://src.chromium.org/svn/trunk/src/net@100742 should_process: True requirements: set(['./']) name: tools/data_pack url: http://src.chromium.org/svn/trunk/src/tools/data_pack@100742 parsed_url: http://src.chromium.org/svn/trunk/src/tools/data_pack@100742 should_process: True requirements: set(['./']) name: third_party/ffmpeg url: From('chromium_deps', 'src/third_party/ffmpeg') should_process: True requirements: set(['third_party', 'chromium_deps', './']) name: tools/generate_stubs url: http://src.chromium.org/svn/trunk/src/tools/generate_stubs@100742 parsed_url: http://src.chromium.org/svn/trunk/src/tools/generate_stubs@100742 should_process: True requirements: set(['./']) name: third_party/libjpeg_turbo url: From('chromium_deps', 'src/third_party/libjpeg_turbo') should_process: True requirements: set(['third_party', 'chromium_deps', './']) name: third_party/v8-i18n url: From('chromium_deps', 'src/third_party/v8-i18n') should_process: True requirements: set(['third_party', 'chromium_deps', './']) name: tools/grit url: http://src.chromium.org/svn/trunk/src/tools/grit@100742 parsed_url: http://src.chromium.org/svn/trunk/src/tools/grit@100742 should_process: True requirements: set(['./']) name: base url: http://src.chromium.org/svn/trunk/src/base@100742 should_process: True requirements: set(['./']) name: sql url: http://src.chromium.org/svn/trunk/src/sql@100742 should_process: True requirements: set(['./']) name: v8 url: From('chromium_deps', 'src/v8') should_process: True requirements: set(['chromium_deps', './']) name: testing/gtest url: From('chromium_deps', 'src/testing/gtest') should_process: True requirements: set(['testing', 'chromium_deps', './']) name: third_party/libwebp url: http://src.chromium.org/svn/trunk/src/third_party/libwebp@100742 should_process: True requirements: set(['third_party', './']) name: googleurl url: From('chromium_deps', 'src/googleurl') should_process: True requirements: set(['chromium_deps', './']) name: third_party/skia/include url: From('chromium_deps', 'src/third_party/skia/include') should_process: True requirements: set(['third_party', 'chromium_deps', './']) name: third_party/ots url: From('chromium_deps', 'src/third_party/ots') should_process: True requirements: set(['third_party', 'chromium_deps', './']) name: third_party/snappy/src url: From('chromium_deps', 'src/third_party/snappy/src') should_process: True requirements: set(['third_party', 'chromium_deps', './']) Died at Tools/Scripts/update-webkit-chromium line 80. No such file or directory at Tools/Scripts/update-webkit line 104. If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 107352 [details] Patch that passes style queue. Hyatt and I went over the changes in person.

Trying again in r95099.