Bug 118665 - [CSS Regions] Implement visual overflow for first & last regions
Summary: [CSS Regions] Implement visual overflow for first & last regions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Radu Stavila
URL:
Keywords: AdobeTracked
: 134302 134457 (view as bug list)
Depends on: 121828 124423
Blocks: 57312 116295 124281 117329 124273
  Show dependency treegraph
 
Reported: 2013-07-15 03:43 PDT by Radu Stavila
Modified: 2014-07-02 08:52 PDT (History)
17 users (show)

See Also:


Attachments
Test-case (2.82 KB, text/html)
2013-07-15 03:43 PDT, Radu Stavila
no flags Details
Patch (70.37 KB, patch)
2013-07-19 05:47 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch (70.38 KB, patch)
2013-07-19 06:12 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch integrating feedback (70.39 KB, patch)
2013-07-19 06:27 PDT, Radu Stavila
hyatt: review-
hyatt: commit-queue-
Details | Formatted Diff | Diff
New approach - WIP (198.82 KB, patch)
2013-09-03 08:09 PDT, Radu Stavila
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (1.80 MB, application/zip)
2013-09-03 12:51 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (1.20 MB, application/zip)
2013-09-04 01:26 PDT, Build Bot
no flags Details
WIP - source files only (tests still failing, ignore) (48.95 KB, application/octet-stream)
2013-09-05 08:42 PDT, Radu Stavila
no flags Details
WIP - source files only (tests still failing, ignore) (48.95 KB, patch)
2013-09-05 08:55 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch (src only, will add tests in a separate attachment, for easier review). (102.95 KB, patch)
2013-11-08 05:49 PST, Radu Stavila
hyatt: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Complete patch (202.27 KB, patch)
2013-11-08 06:00 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (1.43 MB, application/zip)
2013-11-08 06:43 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (1.53 MB, application/zip)
2013-11-08 07:46 PST, Build Bot
no flags Details
Patch integrating reviewer feedback (203.37 KB, patch)
2013-11-13 10:01 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Added automation for testing selection in regions overflow (205.48 KB, patch)
2013-11-14 07:15 PST, Radu Stavila
hyatt: review+
Details | Formatted Diff | Diff
Patch recreated with --binary and updated new repaint test (258.03 KB, patch)
2013-11-14 07:47 PST, Radu Stavila
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (781.72 KB, application/zip)
2013-11-14 09:52 PST, Build Bot
no flags Details
Fixed automated selection test for both WK1 and WK2 (258.17 KB, patch)
2013-11-14 10:44 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Added "reviewed by" in CL (258.16 KB, patch)
2013-11-15 02:09 PST, Radu Stavila
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (258.16 KB, patch)
2013-11-15 07:00 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch with fix for GTK (208.87 KB, patch)
2013-11-20 07:16 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Recreated with --binary (260.17 KB, patch)
2013-11-20 07:38 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Rebased CL (260.10 KB, patch)
2013-11-20 07:43 PST, Radu Stavila
no flags Details | Formatted Diff | Diff
Updated TestExpectations for GTK/EFL, patch for landing (263.31 KB, patch)
2013-11-20 09:09 PST, Radu Stavila
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Radu Stavila 2013-07-15 03:43:16 PDT
Created attachment 206653 [details]
Test-case

There are 3 problems in the attached test:
- the overflow in the 1st named flow is cut at the tile border
- the overflow in the 2nd named flow is not visible due to the region having position:relative and as such, a self-painting layer
- the overflow is not selectable
Comment 1 Radu Stavila 2013-07-19 05:47:41 PDT
Created attachment 207082 [details]
Patch
Comment 2 Mihai Maerean 2013-07-19 06:07:52 PDT
Comment on attachment 207082 [details]
Patch

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

> Source/WebCore/rendering/InlineFlowBox.h:108
> +    void clearKnownToHaveNoOverflowForSelfAndChildren() OVERRIDE;

Please add "virtual" at the beginning of the line.
Comment 3 Radu Stavila 2013-07-19 06:09:21 PDT
Right, missed that, thanks.
Comment 4 Radu Stavila 2013-07-19 06:12:42 PDT
Created attachment 207085 [details]
Patch
Comment 5 Mihai Maerean 2013-07-19 06:18:39 PDT
Comment on attachment 207085 [details]
Patch

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

> Source/WebCore/rendering/RenderFlowThread.cpp:380
> +    if (!RenderBox::addVisualOverflow(rect))

RenderFlowThread derives from RenderBlock. If RenderBlock will override addVisualOverflow, this call will skip that code and will only call the method from RenderBox.
Please change this to RenderBlock::addVisualOverflow.

> Source/WebCore/rendering/RenderRegion.cpp:107
> +    if (!RenderBox::addVisualOverflow(rect))

RenderRegion derives from RenderBlock. If RenderBlock will override addVisualOverflow, this call will skip that code and will only call the method from RenderBox.
Please change this to RenderBlock::addVisualOverflow.
Comment 6 Radu Stavila 2013-07-19 06:27:34 PDT
Created attachment 207087 [details]
Patch integrating feedback
Comment 7 Dave Hyatt 2013-07-19 10:12:30 PDT
Comment on attachment 207087 [details]
Patch integrating feedback

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

r- for a number of writing-mode issues and the weird parent walk that could just use containing blocks.

I think you might find that you could use a simpler approach here though. Instead of trying to write your own propagation code, I think instead you should just mark RenderRegions for a simplified layout pass if you detect that the RenderFlow has overflow that needs to propagate. If you do that, then the layout code can just handle it. You can have RenderRegion when it computes its overflow ask the flow thread if it's in a good state to hand the overflow back and if so, get it from the flow thread.

I think this will be much less code, and you won't have to add all this code to try to propagate outside of layout etc. It also won't work anyway, since the layer code has cached some overflow stuff that you're not fixing up.

> Source/WebCore/rendering/RenderLayer.cpp:4434
> +    LayoutRect hitTestArea;
> +    if (isOutOfFlowRenderFlowThread()) {
> +        RenderFlowThread* renderFlowThread = toRenderFlowThread(renderer());
> +        hitTestArea = renderFlowThread->hasVisualOverflow() ? renderFlowThread->visualOverflowRect() : renderFlowThread->borderBoxRect();
> +    } else
> +        hitTestArea = renderer()->view()->documentRect();
> +    

This is wrong. Overflow is not physical. It is in the coordinate space of the block, so you can't use visualOverflowRect without flipping it for writing mode.

> Source/WebCore/rendering/RenderRegion.cpp:103
> +void RenderRegion::addVisualOverflowFromFlowThreadOverflow()
> +{
> +    LayoutRect flowThreadRect = flowThreadPortionOverflowRect();
> +    flowThreadRect.expand(borderLeft(), borderTop());
> +    flowThreadRect.expand(paddingLeft(), paddingTop());
> +    addVisualOverflow(flowThreadRect);
> +}

This is not correct either. In the case of flipped blocks, the border and padding can be on the bottom and right.

Also, if we're going to support the writing mode of the region differing from the writing mode of the flow thread (do we support that now?), then this code isn't going to be correct either. I don't think you necessarily have to get that right, but you should at least switch to using the correct border sides for flipped block writing modes.

> Source/WebCore/rendering/RenderRegion.cpp:120
> +        RenderObject* objParent = renderChild->parent();
> +        ASSERT(objParent);
> +        
> +        // Skip non-box parents (for instance a region sitting inside a span).
> +        while (!objParent->isBox())
> +            objParent = objParent->parent();

Just use containingBlock(). Overflow follows the containing block hierarchy. This will simplify all of this code.
Comment 8 Dave Hyatt 2013-07-19 10:16:23 PDT
To elaborate, look into  simplifiedNormalFlowLayout and the corresponding bits. All simplifiedNormalFlowLayout does in the most basic cases is recompute overflow, so I think you could leverage it, and then let the normal simplifiedlayout process handle the overflow propagation up the RenderRegion containing block chain. Then all you have to do in RenderRegion is make sure its compute overflow method grabs the overflow it needs from the flow thread if the flow thread is in a good state (which it will be by the time you do the simplified layout).
Comment 9 Dave Hyatt 2013-07-19 10:37:09 PDT
Also, you should be propagating all overflow and not just visual overflow. Layout overflow needs to propagate too.
Comment 10 Radu Stavila 2013-07-19 13:44:51 PDT
Layout overflow is being implemented as we speak in a different patch.
Comment 11 Radu Stavila 2013-07-19 13:47:10 PDT
So, would it be ok for now to implement the changes you requested (the writing direction issues and the parent walk) or just scrap it all and do it the other way, using simplified layout?
Comment 12 Dave Hyatt 2013-07-19 13:48:54 PDT
I would like to see the simplified layout approach used instead. I think it's fragile to have a separate code path for overflow propagation, since that's just another piece of code we'd have to maintain and patch.
Comment 13 Radu Stavila 2013-09-03 08:09:58 PDT
Created attachment 210374 [details]
New approach - WIP
Comment 14 Radu Stavila 2013-09-03 09:26:08 PDT
There are still some failing tests, this is just a WIP. Please review the approach.
Comment 15 Build Bot 2013-09-03 12:51:05 PDT
Comment on attachment 210374 [details]
New approach - WIP

Attachment 210374 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1693340

New failing tests:
compositing/contents-opaque/overflow-hidden-child-layers.html
fast/multicol/transform-inside-opacity.html
compositing/geometry/foreground-layer.html
fast/multicol/newmulticol/float-multicol.html
fast/clip/009.html
fast/regions/overflow-last-region-vert-lr.html
fast/multicol/newmulticol/float-paginate.html
fast/multicol/newmulticol/cell-shrinkback.html
http/tests/inspector/inspect-element.html
fast/block/colspan-under-button-crash.html
fast/multicol/newmulticol/hide-box-horizontal-bt.html
fast/regions/overflow-last-region-horiz-bt.html
fast/multicol/newmulticol/float-avoidance.html
fast/flexbox/clear-overflow-before-scroll-update.html
fast/regions/counters/extract-ordered-lists-in-regions-explicit-counters-005.html
fast/multicol/newmulticol/float-paginate-complex.html
fast/clip/010.html
fast/clip/011.html
fast/clip/012.html
fast/multicol/newmulticol/float-paginate-empty-lines.html
fast/regions/overflow-last-region-vert-rl.html
fast/multicol/newmulticol/layers-in-multicol.html
fast/forms/input-placeholder-text-indent.html
fast/multicol/newmulticol/direct-child-column-span-all.html
css3/flexbox/button.html
fast/multicol/newmulticol/columns-shorthand-parsing.html
compositing/geometry/clipping-foreground.html
fast/forms/input-placeholder-paint-order-2.html
fast/regions/bottom-overflow-out-of-first-region-absolute.html
fast/multicol/newmulticol/column-rules-fixed-height.html
Comment 16 Build Bot 2013-09-03 12:51:09 PDT
Created attachment 210404 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 17 Build Bot 2013-09-04 01:26:49 PDT
Comment on attachment 210374 [details]
New approach - WIP

Attachment 210374 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1683012

New failing tests:
fast/multicol/newmulticol/columns-shorthand-parsing.html
compositing/geometry/clipping-foreground.html
compositing/contents-opaque/overflow-hidden-child-layers.html
compositing/geometry/foreground-layer.html
fast/multicol/newmulticol/column-rules-fixed-height.html
fast/multicol/newmulticol/direct-child-column-span-all.html
fast/clip/009.html
fast/multicol/newmulticol/float-avoidance.html
fast/flexbox/clear-overflow-before-scroll-update.html
fast/multicol/newmulticol/float-multicol.html
http/tests/misc/submit-post-keygen.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
fast/multicol/newmulticol/cell-shrinkback.html
fast/clip/010.html
css3/flexbox/button.html
http/tests/inspector/inspect-element.html
fast/clip/011.html
Comment 18 Build Bot 2013-09-04 01:26:53 PDT
Created attachment 210438 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.4
Comment 19 Radu Stavila 2013-09-05 08:42:04 PDT
Created attachment 210620 [details]
WIP - source files only (tests still failing, ignore)
Comment 20 Radu Stavila 2013-09-05 08:55:39 PDT
Created attachment 210621 [details]
WIP - source files only (tests still failing, ignore)
Comment 21 Dave Hyatt 2013-09-10 10:00:58 PDT
Comment on attachment 210621 [details]
WIP - source files only (tests still failing, ignore)

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

> Source/WebCore/rendering/RenderBlock.cpp:3351
> +    // Check our region range to make sure we need to be painting in this region.
> +    if (paintInfo.renderRegion && !paintInfo.renderRegion->flowThread()->objectInFlowRegion(this, paintInfo.renderRegion))
> +        return;

Can't you catch this earlier?

> Source/WebCore/rendering/RenderBox.cpp:2084
> +    if (o->isRenderFlowThread()) {
> +        RenderRegion* firstRegion = 0;
> +        RenderRegion* lastRegion = 0;
> +        toRenderFlowThread(o)->getBoxRegionRangeForOverflow(this, firstRegion, lastRegion);
> +        if (firstRegion)
> +            rect.moveBy(firstRegion->flowThreadPortionRect().location());
> +    }

Probably other places you will need to patch.

> Source/WebCore/rendering/RenderLayer.cpp:482
>          if (flags & CheckForRepaint) {
>              if (!renderer().view().printing()) {
> +                bool repaintRegion = false;
>                  if (m_repaintStatus & NeedsFullRepaint) {
>                      renderer().repaintUsingContainer(repaintContainer, pixelSnappedIntRect(oldRepaintRect));
> -                    if (m_repaintRect != oldRepaintRect)
> +                    if (m_repaintRect != oldRepaintRect) {
>                          renderer().repaintUsingContainer(repaintContainer, pixelSnappedIntRect(m_repaintRect));
> -                } else if (shouldRepaintAfterLayout())
> +                        
> +                        if (renderer().isRenderRegion())
> +                            repaintRegion = true;
> +                    }
> +                } else if (shouldRepaintAfterLayout()) {
>                      renderer().repaintAfterLayoutIfNeeded(repaintContainer, oldRepaintRect, oldOutlineBox, &m_repaintRect, &m_outlineBox);
> +
> +                    if (renderer().isRenderRegion())
> +                        repaintRegion = true;
> +                }
> +
> +                // If we just repainted a region, we must also repaint the flow thread since it is the one
> +                // doing the actual painting of the flowed content.
> +                if (repaintRegion) {
> +                    RenderRegion* region = toRenderRegion(&renderer());
> +                    if (region->flowThread())
> +                        region->flowThread()->layer()->repaintIncludingDescendants();
> +                }

Maybe think about doing the region work in a helper function, even if it duplicates a couple of lines of code.

> Source/WebCore/rendering/RenderLayer.cpp:1658
> +    // If this is a region, then the painting is actually done by its flow thread's layer.
> +    if (layer->renderer().isRenderRegion()) {
> +        RenderRegion* region = toRenderRegion(&layer->renderer());
> +        RenderLayer* flowThreadLayer = region->flowThread()->layer();
> +        if (!layer->reflection() || layer->reflectionLayer() != flowThreadLayer) {
> +            LayoutRect flowThreadClipRect = transparencyClipBox(flowThreadLayer, rootLayer, transparencyBehavior, DescendantsOfTransparencyClipBox, paintBehavior);
> +            
> +            LayoutPoint offsetFromRoot;
> +            layer->convertToLayerCoords(flowThreadLayer, offsetFromRoot);
> +
> +            LayoutSize moveOffset = (offsetFromRoot + region->contentBoxRect().location()) - region->flowThreadPortionRect().location();
> +            flowThreadClipRect.move(moveOffset);
> +            
> +            clipRect.unite(flowThreadClipRect);
> +        }
> +    }

Big enough to warrant a helper function I think.

> Source/WebCore/rendering/RenderLayer.cpp:4002
> +            ClipRect regionClipRect;
> +            if (localPaintingInfo.rootLayer != this && parent()) {
> +                ClipRectsContext clipRectsContext(localPaintingInfo.rootLayer, toRenderRegion(&renderer()),
> +                    (localPaintFlags & PaintLayerTemporaryClipRects) ? TemporaryClipRects : PaintingClipRects,
> +                    IgnoreOverlayScrollbarSize, (isPaintingOverflowContents) ? IgnoreOverflowClip : RespectOverflowClip);
> +                regionClipRect = backgroundClipRect(clipRectsContext);
> +            } else
> +                regionClipRect = paintDirtyRect;
> +
> +            paintFlowThreadForRegion(context, localPaintingInfo, localPaintFlags, offsetFromRoot, regionClipRect);
> +        }

Helper function.
Comment 22 Radu Stavila 2013-11-08 05:49:20 PST
Created attachment 216380 [details]
Patch (src only, will add tests in a separate attachment, for easier review).
Comment 23 Radu Stavila 2013-11-08 06:00:03 PST
Created attachment 216383 [details]
Complete patch
Comment 24 Build Bot 2013-11-08 06:43:24 PST
Comment on attachment 216380 [details]
Patch (src only, will add tests in a separate attachment, for easier review).

Attachment 216380 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/22738477

New failing tests:
fast/regions/element-in-named-flow-fixed-from-absolute.html
fast/regions/top-overflow-out-of-second-region.html
fast/regions/float-pushed-width-change-2.html
fast/regions/webkit-flow-float-unable-to-push.html
fast/regions/bottom-overflow-out-of-first-region.html
fast/regions/element-outflow-static-from-inflow-fixed.html
fast/regions/element-inflow-fixed-from-outflow-static.html
fast/regions/counters/extract-ordered-lists-in-regions-explicit-counters-005.html
fast/regions/float-pushed-width-change.html
fast/regions/element-in-named-flow-absolute-from-fixed.html
fast/regions/overflow-scrollable-rotated-fragment.html
Comment 25 Build Bot 2013-11-08 06:43:28 PST
Created attachment 216390 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 26 Build Bot 2013-11-08 07:46:10 PST
Comment on attachment 216380 [details]
Patch (src only, will add tests in a separate attachment, for easier review).

Attachment 216380 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/22508538

New failing tests:
fast/regions/element-in-named-flow-fixed-from-absolute.html
fast/regions/top-overflow-out-of-second-region.html
fast/regions/float-pushed-width-change-2.html
fast/regions/webkit-flow-float-unable-to-push.html
fast/regions/bottom-overflow-out-of-first-region.html
fast/regions/element-outflow-static-from-inflow-fixed.html
fast/regions/element-inflow-fixed-from-outflow-static.html
fast/regions/counters/extract-ordered-lists-in-regions-explicit-counters-005.html
fast/regions/float-pushed-width-change.html
fast/regions/element-in-named-flow-absolute-from-fixed.html
fast/regions/overflow-scrollable-rotated-fragment.html
Comment 27 Build Bot 2013-11-08 07:46:13 PST
Created attachment 216397 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 28 Dave Hyatt 2013-11-12 11:27:16 PST
Comment on attachment 216380 [details]
Patch (src only, will add tests in a separate attachment, for easier review).

This patch looks like it is breaking overflow for multicolumn, so that's the reason for the minus. Your options are basically to try to preserve the original behavior, or to just make the change to actually do overflow for columns. Columns were basically using the flow thread overflow and intersecting that with a rect that extended halfway into the column gap for interior columns. The repaintFlowThreadContentRectangle handled this, since you could pass in an overflow rect for repainting and have the right thing happen.

If you decide you really want to try to do visual overflow for columns, the tricky bit is that each column needs a RenderOverflow, and not just the single RenderMultiColumnSet as a whole. The overflow portion rect in the flow thread is the same as per a region with the addition of the "clip halfway into the column gap" rule for interior edges (so first and last column don't have their exterior edges clipped).

I'm fine with overflow not being implemented for columns as long as you don't break it though.
Comment 29 Radu Stavila 2013-11-13 07:25:34 PST
The repaintRect parameter passed to the repaintFlowThreadContentRectangle already contains the flow thread's overflow in this situation. I tested the repainting of region-based multicol by setting negative top-margin on the content in the multicol and moving the multicol around on :hover. Everything works the same in both versions (with and without the patch). Is there a particular scenario or test-case that you have in mind on which to test this issue?

Thanks.
Comment 30 Radu Stavila 2013-11-13 10:01:05 PST
Created attachment 216816 [details]
Patch integrating reviewer feedback
Comment 31 Radu Stavila 2013-11-14 07:15:01 PST
Created attachment 216931 [details]
Added automation for testing selection in regions overflow
Comment 32 Dave Hyatt 2013-11-14 07:37:40 PST
Comment on attachment 216931 [details]
Added automation for testing selection in regions overflow

r=me
Comment 33 Radu Stavila 2013-11-14 07:47:10 PST
Created attachment 216934 [details]
Patch recreated with --binary and updated new repaint test
Comment 34 Build Bot 2013-11-14 09:52:47 PST
Comment on attachment 216934 [details]
Patch recreated with --binary and updated new repaint test

Attachment 216934 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/23618009

New failing tests:
fast/regions/overflow-nested-regions.html
Comment 35 Build Bot 2013-11-14 09:52:51 PST
Created attachment 216955 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 36 Radu Stavila 2013-11-14 10:44:25 PST
Created attachment 216960 [details]
Fixed automated selection test for both WK1 and WK2
Comment 37 Radu Stavila 2013-11-15 02:09:20 PST
Created attachment 217028 [details]
Added "reviewed by" in CL
Comment 38 WebKit Commit Bot 2013-11-15 05:06:09 PST
Comment on attachment 217028 [details]
Added "reviewed by" in CL

Rejecting attachment 217028 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 217028, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Dave Hyatt found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/23958080
Comment 39 Radu Stavila 2013-11-15 07:00:16 PST
Created attachment 217047 [details]
Patch for landing
Comment 40 WebKit Commit Bot 2013-11-15 07:32:09 PST
Comment on attachment 217047 [details]
Patch for landing

Clearing flags on attachment: 217047

Committed r159337: <http://trac.webkit.org/changeset/159337>
Comment 41 Philippe Normand 2013-11-15 11:18:25 PST
Sorry but I had to roll this patch out as it broke tests on EFL and GTK. I am CCing Rego and Javi Fernandez, they might be able to help fix the issue on GTK at least.
Comment 42 Javier Fernandez 2013-11-16 07:47:41 PST
(In reply to comment #41)
> Sorry but I had to roll this patch out as it broke tests on EFL and GTK. I am CCing Rego and Javi Fernandez, they might be able to help fix the issue on GTK at least.

I'll take care of it.
Comment 43 Radu Stavila 2013-11-20 07:16:29 PST
Created attachment 217426 [details]
Patch with fix for GTK
Comment 44 Radu Stavila 2013-11-20 07:38:29 PST
Created attachment 217430 [details]
Recreated with --binary
Comment 45 Radu Stavila 2013-11-20 07:43:52 PST
Created attachment 217432 [details]
Rebased CL
Comment 46 Radu Stavila 2013-11-20 09:09:21 PST
Created attachment 217440 [details]
Updated TestExpectations for GTK/EFL, patch for landing
Comment 47 WebKit Commit Bot 2013-11-20 22:54:56 PST
Comment on attachment 217440 [details]
Updated TestExpectations for GTK/EFL, patch for landing

Clearing flags on attachment: 217440

Committed r159609: <http://trac.webkit.org/changeset/159609>
Comment 48 Jon Lee 2014-06-30 11:10:54 PDT
*** Bug 134302 has been marked as a duplicate of this bug. ***
Comment 49 Simon Fraser (smfr) 2014-07-02 08:52:21 PDT
*** Bug 134457 has been marked as a duplicate of this bug. ***