RESOLVED FIXED 94198
fast/dom/HTMLImageElement/image-alt-text.html and fast/dom/HTMLInputElement/input-image-alt-text.html are failing
https://bugs.webkit.org/show_bug.cgi?id=94198
Summary fast/dom/HTMLImageElement/image-alt-text.html and fast/dom/HTMLInputElement/i...
Chris Dumez
Reported 2012-08-16 00:40:02 PDT
The following test cases seem to be failing for all the ports: fast/dom/HTMLImageElement/image-alt-text.html fast/dom/HTMLInputElement/input-image-alt-text.html Those tests are not skipped because the ports seem to have wrong expected results: - The text clearly says that "SUCCESS" is supposed to be printed twice but it is printed only once in the expected results According to HTML5 spec, the alt text MUST be displayed for images that don't have "src" attribute set. However, for those 2 test cases, we are not displaying the alt text for the first image (which does not have src set).
Attachments
Patch (572.77 KB, patch)
2012-08-16 04:36 PDT, Chris Dumez
no flags
Patch (573.64 KB, patch)
2012-08-16 04:59 PDT, Chris Dumez
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-02 (1.39 MB, application/zip)
2012-08-16 05:47 PDT, WebKit Review Bot
no flags
Patch (576.19 KB, patch)
2012-08-16 15:05 PDT, Chris Dumez
webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-06 (1.16 MB, application/zip)
2012-08-16 17:25 PDT, WebKit Review Bot
no flags
Patch (1.25 MB, patch)
2012-08-16 22:51 PDT, Chris Dumez
no flags
Archive of layout-test-results from gce-cr-linux-07 (1.42 MB, application/zip)
2012-08-17 01:36 PDT, WebKit Review Bot
no flags
Patch (966.01 KB, patch)
2013-01-23 12:57 PST, Chris Dumez
webkit.review.bot: commit-queue-
Patch (1.13 MB, patch)
2013-01-23 23:53 PST, Chris Dumez
no flags
Patch (1.13 MB, patch)
2013-01-29 23:17 PST, Chris Dumez
darin: review+
webkit.review.bot: commit-queue-
Patch for landing (1.16 MB, patch)
2013-04-02 11:43 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2012-08-16 00:52:36 PDT
Firefox displays the alt text properly for those 2 test cases.
Raphael Kubo da Costa (:rakuco)
Comment 2 2012-08-16 01:04:28 PDT
For the record, the text is not being displayed because the size of the element is too small for the text to appear; making it a few pixels taller than the default usually makes it be shown.
Chris Dumez
Comment 3 2012-08-16 01:51:59 PDT
The following happens: 1. setImageSizeForAltText() and intrinsic size is calculated like this: IntSize textSize(min(font.width(RenderBlock::constructTextRun(this, font, m_altText, style())), maxAltTextWidth), min(font.fontMetrics().height(), maxAltTextHeight)); // setImageSizeForAltText(): new size: (65, 19) 2. RenderImage::paintReplaced() is called: // cWidth: 65, cHeight: 19 Then an outline rect is drawn where the image should be and the following is done: // When calculating the usable dimensions, exclude the pixels of // the ouline rect so the error image/alt text doesn't draw on it. LayoutUnit usableWidth = cWidth - 2; LayoutUnit usableHeight = cHeight - 2; Then, we check if the usableWidth / cHeight is sufficient to print the alt text: if (usableWidth >= textWidth && cHeight >= fontMetrics.height()) context->drawText(font, textRun, altTextOffset); For some reason we use usableWidth instead of cWidth although we use cHeight instead of usableHeight. As a consequence, the check fails and the text is not drawn: // usableWidth: 63, textWidth: 65, cHeight: 19, fontMetrics.height(): 19 usableWidth is not enough to fit 65 because of the outline border width.
Chris Dumez
Comment 4 2012-08-16 04:36:08 PDT
Created attachment 158772 [details] Patch Unfortunately, this requires rebaselining for 6 tests for all the ports. My patch does the rebaselining for EFL port only.
Kenneth Rohde Christiansen
Comment 5 2012-08-16 04:42:15 PDT
Comment on attachment 158772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158772&action=review > Source/WebCore/ChangeLog:14 > + Fix alt text not being displayed for img elements > + or input of type "image" due to insufficient size. > + > + No new tests, already covered by existing tests. > + > + * rendering/RenderImage.cpp: > + (WebCore::RenderImage::setImageSizeForAltText): It would be good if you said shortly what the issue was and how you fixed it. Especially to convince people that it is the right approach
Chris Dumez
Comment 6 2012-08-16 04:59:20 PDT
Created attachment 158778 [details] Patch Improve Changelog explanation.
WebKit Review Bot
Comment 7 2012-08-16 05:47:33 PDT
Comment on attachment 158778 [details] Patch Attachment 158778 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13519022 New failing tests: fast/encoding/utf-16-big-endian.html fast/block/float/002.html fast/parser/comment-in-script.html fast/encoding/utf-16-little-endian.html tables/mozilla/bugs/bug2997.html fast/dom/HTMLInputElement/input-image-alt-text.html editing/selection/select-missing-image.html fast/invalid/012.html fast/forms/input-value.html fast/block/float/017.html fast/flexbox/024.html tables/mozilla/collapsing_borders/bug41262-3.html fast/dom/HTMLImageElement/image-alt-text.html fast/flexbox/023.html
WebKit Review Bot
Comment 8 2012-08-16 05:47:41 PDT
Created attachment 158786 [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
Levi Weintraub
Comment 9 2012-08-16 11:38:20 PDT
Comment on attachment 158778 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158778&action=review You'll want to set proper test expectations for other ports so you don't turn the bots red. > LayoutTests/ChangeLog:9 > + Provide new baseline for several tests that are displaying text > + text for img or input elements now that their size is "text text" > Source/WebCore/rendering/RenderImage.cpp:121 > + IntSize paddedTextSize(paddingWidth + min(font.width(RenderBlock::constructTextRun(this, font, m_altText, style())), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); Do you really want to truncate the floating point width of the text? Shouldn't it be ceiled to contain? > Source/WebCore/rendering/RenderImage.cpp:342 > + altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - 1, topBorder + topPad + ascent + (paddingHeight / 2) - 1); Why the -1? (paddingWidth / 2) - 1 will just always be 1, right?
Robert Hogan
Comment 10 2012-08-16 11:46:43 PDT
Chris Dumez
Comment 11 2012-08-16 11:53:45 PDT
(In reply to comment #9) > (From update of attachment 158778 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158778&action=review > > You'll want to set proper test expectations for other ports so you don't turn the bots red. Hmm. It is not going to be easy for me to generated expected results for other ports. I can easily do it for GTK port but not sure about others. > > LayoutTests/ChangeLog:9 > > + Provide new baseline for several tests that are displaying text > > + text for img or input elements now that their size is > > "text text" Will fix this thanks. > > Source/WebCore/rendering/RenderImage.cpp:121 > > + IntSize paddedTextSize(paddingWidth + min(font.width(RenderBlock::constructTextRun(this, font, m_altText, style())), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); > > Do you really want to truncate the floating point width of the text? Shouldn't it be ceiled to contain? I don't really understand your comment. I'm merely adding paddingWidth to the text width and paddingHeight to the text height. I did not change any truncating behavior as far as I understand. > > Source/WebCore/rendering/RenderImage.cpp:342 > > + altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - 1, topBorder + topPad + ascent + (paddingHeight / 2) - 1); > > Why the -1? > > (paddingWidth / 2) - 1 will just always be 1, right? Yes, the result will always be 1 but I decided to use the formula since it relies on paddingWidth and it is less easily breakable this way. The -1 is for the outline border since we start from leftBorder: The outline rect width is equal to TextWidth + paddingWidth The outline rect has a 1px border (inside the rectangle, right?) Therefore, the border width is included in the padding and must be substracted I believe.
Levi Weintraub
Comment 12 2012-08-16 11:58:35 PDT
Comment on attachment 158778 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158778&action=review >>> Source/WebCore/rendering/RenderImage.cpp:121 >>> + IntSize paddedTextSize(paddingWidth + min(font.width(RenderBlock::constructTextRun(this, font, m_altText, style())), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); >> >> Do you really want to truncate the floating point width of the text? Shouldn't it be ceiled to contain? > > I don't really understand your comment. I'm merely adding paddingWidth to the text width and paddingHeight to the text height. I did not change any truncating behavior as far as I understand. font.width returns a float. You're attempting to determine the size of the text then assigning it to an integer, which will lose precision and can result in a width value that's less than the actual width of the text. I'm not saying you're regressing, but the current behavior looks wrong. >>> Source/WebCore/rendering/RenderImage.cpp:342 >>> + altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - 1, topBorder + topPad + ascent + (paddingHeight / 2) - 1); >> >> Why the -1? >> >> (paddingWidth / 2) - 1 will just always be 1, right? > > Yes, the result will always be 1 but I decided to use the formula since it relies on paddingWidth and it is less easily breakable this way. > > The -1 is for the outline border since we start from leftBorder: > The outline rect width is equal to TextWidth + paddingWidth > The outline rect has a 1px border (inside the rectangle, right?) > > Therefore, the border width is included in the padding and must be substracted I believe. The formula is fine, but the -1 doesn't communicate where it comes from. If that's us subtracting the outline border, how about a named constant that communicates this?
Chris Dumez
Comment 13 2012-08-16 12:04:19 PDT
(In reply to comment #12) > (From update of attachment 158778 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158778&action=review > > >>> Source/WebCore/rendering/RenderImage.cpp:121 > >>> + IntSize paddedTextSize(paddingWidth + min(font.width(RenderBlock::constructTextRun(this, font, m_altText, style())), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); > >> > >> Do you really want to truncate the floating point width of the text? Shouldn't it be ceiled to contain? > > > > I don't really understand your comment. I'm merely adding paddingWidth to the text width and paddingHeight to the text height. I did not change any truncating behavior as far as I understand. > > font.width returns a float. You're attempting to determine the size of the text then assigning it to an integer, which will lose precision and can result in a width value that's less than the actual width of the text. I'm not saying you're regressing, but the current behavior looks wrong. Right. I will ceil it then. > >>> Source/WebCore/rendering/RenderImage.cpp:342 > >>> + altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - 1, topBorder + topPad + ascent + (paddingHeight / 2) - 1); > >> > >> Why the -1? > >> > >> (paddingWidth / 2) - 1 will just always be 1, right? > > > > Yes, the result will always be 1 but I decided to use the formula since it relies on paddingWidth and it is less easily breakable this way. > > > > The -1 is for the outline border since we start from leftBorder: > > The outline rect width is equal to TextWidth + paddingWidth > > The outline rect has a 1px border (inside the rectangle, right?) > > > > Therefore, the border width is included in the padding and must be substracted I believe. > > The formula is fine, but the -1 doesn't communicate where it comes from. If that's us subtracting the outline border, how about a named constant that communicates this? Agreed. A named constant would make this formula clearer.
Chris Dumez
Comment 14 2012-08-16 15:05:58 PDT
Created attachment 158916 [details] Patch Take Levi's feedback into consideration (except for other ports' rebaselining).
WebKit Review Bot
Comment 15 2012-08-16 17:25:11 PDT
Comment on attachment 158916 [details] Patch Attachment 158916 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13514581 New failing tests: fast/encoding/utf-16-big-endian.html fast/block/float/002.html fast/parser/comment-in-script.html fast/encoding/utf-16-little-endian.html tables/mozilla/bugs/bug2997.html fast/dom/HTMLInputElement/input-image-alt-text.html editing/selection/select-missing-image.html fast/invalid/012.html fast/forms/input-value.html fast/block/float/017.html fast/flexbox/024.html tables/mozilla/collapsing_borders/bug41262-3.html fast/dom/HTMLImageElement/image-alt-text.html fast/flexbox/023.html
WebKit Review Bot
Comment 16 2012-08-16 17:25:18 PDT
Created attachment 158955 [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
Chris Dumez
Comment 17 2012-08-16 22:51:30 PDT
Created attachment 159005 [details] Patch Add new expected results for GTK port.
Chris Dumez
Comment 18 2012-08-16 22:54:49 PDT
Would it be acceptable for other ports than EFL / GTK to simply move the affected test cases to TestExpectations with a comment such as: "Rebaseline needed after Bug 94198" ?
WebKit Review Bot
Comment 19 2012-08-17 01:36:37 PDT
Comment on attachment 159005 [details] Patch Attachment 159005 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13516710 New failing tests: fast/encoding/utf-16-big-endian.html fast/block/float/002.html fast/parser/comment-in-script.html fast/encoding/utf-16-little-endian.html tables/mozilla/bugs/bug2997.html fast/dom/HTMLInputElement/input-image-alt-text.html editing/selection/select-missing-image.html fast/invalid/012.html fast/forms/input-value.html fast/block/float/017.html fast/flexbox/024.html tables/mozilla/collapsing_borders/bug41262-3.html fast/dom/HTMLImageElement/image-alt-text.html fast/flexbox/023.html
WebKit Review Bot
Comment 20 2012-08-17 01:36:45 PDT
Created attachment 159046 [details] Archive of layout-test-results from gce-cr-linux-07 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-07 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Hajime Morrita
Comment 21 2012-08-19 18:15:06 PDT
Comment on attachment 159005 [details] Patch It looks chromium redness is just a false alarm.
Hajime Morrita
Comment 22 2012-08-19 22:57:53 PDT
Comment on attachment 159005 [details] Patch cancelling my r+ since RenderImage part needs a review from some experts.
Darin Adler
Comment 23 2013-01-16 11:30:54 PST
Comment on attachment 159005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159005&action=review Generally the approach looks fine. > Source/WebCore/ChangeLog:8 > + Fix alt text not being displayed for img elements This is wrapped kind of narrow for change log! > Source/WebCore/rendering/RenderImage.cpp:121 > + IntSize paddedTextSize(paddingWidth + min(ceilf(font.width(RenderBlock::constructTextRun(this, font, m_altText, style()))), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); I’m a little surprised by the addition of the call to ceilf here. Nothing in the change log mentions that. > Source/WebCore/rendering/RenderImage.cpp:301 > + static const int borderWidth = 1; The static isn’t needed or helpful here.
Chris Dumez
Comment 24 2013-01-23 10:14:44 PST
Comment on attachment 159005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159005&action=review This patch is a bit old so I will have to regenerate the baselines. >> Source/WebCore/rendering/RenderImage.cpp:121 >> + IntSize paddedTextSize(paddingWidth + min(ceilf(font.width(RenderBlock::constructTextRun(this, font, m_altText, style()))), maxAltTextWidth), paddingHeight + min(font.fontMetrics().height(), maxAltTextHeight)); > > I’m a little surprised by the addition of the call to ceilf here. Nothing in the change log mentions that. This was proposed by Levi Weintraub in https://bugs.webkit.org/show_bug.cgi?id=94198#c9. font.width() returns a float which used to be truncated into an integer. I believe that ceiling the value is the correct way to go here since we want a bounding box. If the text width is 11.5 (and there is no padding), we want the bounding rectangle width to be 12, not 11. I will add this information to the Changelog. >> Source/WebCore/rendering/RenderImage.cpp:301 >> + static const int borderWidth = 1; > > The static isn’t needed or helpful here. Ok.
Chris Dumez
Comment 25 2013-01-23 12:57:34 PST
Created attachment 184290 [details] Patch Take Darin's feedback into consideration. Also regenerate baselines for EFL port and add the test cases to other ports TestExpectations to avoid causing redness.
WebKit Review Bot
Comment 26 2013-01-23 13:01:28 PST
Attachment 184290 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.txt', u'LayoutTests/platform/efl/fast/forms/input-value-expected.png', u'LayoutTests/platform/efl/fast/forms/input-value-expected.txt', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.png', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.txt', u'LayoutTests/platform/efl/fast/text/whitespace/normal-after-nowrap-breaking-expected.png', u'LayoutTests/platform/efl/tables/mozilla/bugs/bug2997-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.txt', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/platform/win/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderImage.cpp']" exit_code: 1 LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-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] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Chris Dumez
Comment 27 2013-01-23 13:13:14 PST
(In reply to comment #26) > Attachment 184290 [details] did not pass style-queue: > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.txt', u'LayoutTests/platform/efl/fast/forms/input-value-expected.png', u'LayoutTests/platform/efl/fast/forms/input-value-expected.txt', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.png', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.txt', u'LayoutTests/platform/efl/fast/text/whitespace/normal-after-nowrap-breaking-expected.png', u'LayoutTests/platform/efl/tables/mozilla/bugs/bug2997-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.txt', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/platform/win/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderImage.cpp']" exit_code: 1 > LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-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] > Total errors found: 1 in 16 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. It seems the svn:mime-type config needs to be fixed on the style bot. I have no style errors locally.
Chris Dumez
Comment 28 2013-01-23 13:17:03 PST
(In reply to comment #27) > (In reply to comment #26) > > Attachment 184290 [details] [details] did not pass style-queue: > > > > Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.txt', u'LayoutTests/platform/efl/fast/forms/input-value-expected.png', u'LayoutTests/platform/efl/fast/forms/input-value-expected.txt', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.png', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.txt', u'LayoutTests/platform/efl/fast/text/whitespace/normal-after-nowrap-breaking-expected.png', u'LayoutTests/platform/efl/tables/mozilla/bugs/bug2997-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.txt', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/platform/win/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderImage.cpp']" exit_code: 1 > > LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-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] > > Total errors found: 1 in 16 files > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > It seems the svn:mime-type config needs to be fixed on the style bot. I have no style errors locally. FYI, a bug was already filed for this style bot issue: Bug 107724.
WebKit Review Bot
Comment 29 2013-01-23 16:08:20 PST
Comment on attachment 184290 [details] Patch Attachment 184290 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16082349 New failing tests: fast/block/float/002.html fast/parser/comment-in-script.html tables/mozilla/bugs/bug2997.html editing/selection/select-missing-image.html fast/invalid/012.html fast/flexbox/024.html fast/block/float/017.html fast/flexbox/023.html
Build Bot
Comment 30 2013-01-23 17:48:56 PST
Comment on attachment 184290 [details] Patch Attachment 184290 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16076489 New failing tests: transitions/mixed-type.html transitions/interrupted-accelerated-transition.html transitions/cubic-bezier-overflow-svg-length.html transitions/mismatched-shadow-styles.html transitions/default-timing-function.html transitions/cubic-bezier-overflow-transform.html transitions/multiple-background-size-transitions.html transitions/mask-transitions.html transitions/negative-delay.html transitions/background-transitions.html transitions/flex-transitions.html transitions/multiple-shadow-transitions.html transitions/mismatched-shadow-transitions.html transitions/color-transition-premultiplied.html transitions/interrupt-zero-duration.html transitions/cross-fade-border-image.html transitions/color-transition-all.html transitions/cubic-bezier-overflow-length.html transitions/multiple-mask-transitions.html fast/lists/inlineBoxWrapperNullCheck.html transitions/multiple-background-transitions.html transitions/cancel-transition.html transitions/cubic-bezier-overflow-shadow.html transitions/cubic-bezier-overflow-color.html transitions/color-transition-rounding.html transitions/border-radius-transition.html transitions/cross-fade-background-image.html transitions/clip-transition.html fast/loader/javascript-url-in-object.html transitions/min-max-width-height-transitions.html
Build Bot
Comment 31 2013-01-23 23:23:20 PST
Comment on attachment 184290 [details] Patch Attachment 184290 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16078582 New failing tests: fast/lists/inlineBoxWrapperNullCheck.html
Chris Dumez
Comment 32 2013-01-23 23:53:58 PST
Created attachment 184418 [details] Patch Should make ews bots happier, marked a few more tests as needing rebaseline on chromium and mac.
WebKit Review Bot
Comment 33 2013-01-23 23:56:38 PST
Attachment 184418 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/fast/block/float/002-expected.png', u'LayoutTests/platform/efl/fast/block/float/017-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLImageElement/image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.png', u'LayoutTests/platform/efl/fast/dom/HTMLInputElement/input-image-alt-text-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-big-endian-expected.txt', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.png', u'LayoutTests/platform/efl/fast/encoding/utf-16-little-endian-expected.txt', u'LayoutTests/platform/efl/fast/flexbox/023-expected.png', u'LayoutTests/platform/efl/fast/flexbox/024-expected.png', u'LayoutTests/platform/efl/fast/forms/input-value-expected.png', u'LayoutTests/platform/efl/fast/forms/input-value-expected.txt', u'LayoutTests/platform/efl/fast/invalid/012-expected.png', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.png', u'LayoutTests/platform/efl/fast/lists/inlineBoxWrapperNullCheck-expected.txt', u'LayoutTests/platform/efl/fast/parser/comment-in-script-expected.png', u'LayoutTests/platform/efl/fast/text/whitespace/normal-after-nowrap-breaking-expected.png', u'LayoutTests/platform/efl/tables/mozilla/bugs/bug2997-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.png', u'LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-expected.txt', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'LayoutTests/platform/win/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderImage.cpp']" exit_code: 1 LayoutTests/platform/efl/tables/mozilla/collapsing_borders/bug41262-3-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] Total errors found: 1 in 16 files If any of these errors are false positives, please file a bug against check-webkit-style.
Chris Dumez
Comment 34 2013-01-24 02:48:50 PST
Ok, it appears the EWS bots are happy now. Darin, would you mind taking another look please?
Chris Dumez
Comment 35 2013-01-29 23:17:18 PST
Eric Seidel (no email)
Comment 36 2013-03-01 10:38:56 PST
Comment on attachment 159005 [details] Patch Cleared Darin Adler's review+ from obsolete attachment 159005 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Chris Dumez
Comment 37 2013-04-02 09:56:47 PDT
Darin, could you please take a look at the latest patch please?
WebKit Review Bot
Comment 38 2013-04-02 10:15:53 PDT
Comment on attachment 185403 [details] Patch Rejecting attachment 185403 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 185403, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: cceeded at 1 with fuzz 3. patching file Source/WebCore/rendering/RenderImage.cpp Hunk #1 succeeded at 127 (offset 1 line). Hunk #2 succeeded at 321 (offset 3 lines). Hunk #3 succeeded at 333 (offset 3 lines). Hunk #4 succeeded at 352 (offset 3 lines). Hunk #5 succeeded at 364 (offset 3 lines). Hunk #6 succeeded at 373 (offset 3 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Darin Adler']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://webkit-commit-queue.appspot.com/results/17384194
Chris Dumez
Comment 39 2013-04-02 11:43:14 PDT
Created attachment 196197 [details] Patch for landing
WebKit Review Bot
Comment 40 2013-04-02 13:34:13 PDT
Comment on attachment 196197 [details] Patch for landing Clearing flags on attachment: 196197 Committed r147492: <http://trac.webkit.org/changeset/147492>
WebKit Review Bot
Comment 41 2013-04-02 13:34:22 PDT
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 42 2013-04-03 00:52:56 PDT
Landed new baselines for GTK port in <http://trac.webkit.org/changeset/147524>.
Chris Dumez
Comment 43 2013-04-03 01:37:36 PDT
Landed new baselines for Chromium in <http://trac.webkit.org/changeset/147527>.
Darin Adler
Comment 44 2016-03-09 09:13:50 PST
*** Bug 5566 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.