Changeset 203522 in webkit


Ignore:
Timestamp:
Jul 21, 2016 2:06:51 PM (8 years ago)
Author:
dbates@webkit.org
Message:

REGRESSION: Plugin replaced YouTube Flash videos always have the same width
​https://bugs.webkit.org/show_bug.cgi?id=159998
<rdar://problem/27462285>

Reviewed by Simon Fraser.

Source/WebCore:

Fixes an issue where the width of a plugin replaced YouTube video loaded via an HTML embed
element would always have the same width regardless of value of the width attribute.

For YouTube Flash videos the YouTube plugin replacement substitutes a shadow DOM subtree
for the default renderer of an HTML embed element. The root of this shadow DOM subtree
is an HTML div element. Currently we set inline styles on this <div> when it is instantiated.
In particular, we set inline display and position to "inline-block" and "relative", respectively,
and set an invalid height and width (we specify a font weight value instead of a CSS length value

  • this causes an ASSERT_NOT_REACHED() assertion failure in StyleBuilderConverter::convertLengthSizing()

in a debug build). These styles never worked as intended and we ultimately created an inline
renderer (ignoring display "inline-block") that had auto width and height. Instead it is sufficient
to remove all these inline styles and create a RenderBlockFlow renderer for this <div> so that it
renders as a block, non-replaced element to achieve the intended illusion that the <embed> is a
single element.

  • html/shadow/YouTubeEmbedShadowElement.cpp: Remove unused header HTMLEmbedElement.h and include

header RenderBlockFlow.h. Also update copyright in license block.
(WebCore::YouTubeEmbedShadowElement::YouTubeEmbedShadowElement): Remove inline styles as these
never worked as intended.
(WebCore::YouTubeEmbedShadowElement::createElementRenderer): Override; create a block-flow
renderer for us so that we layout as a block, non-replaced element.

  • html/shadow/YouTubeEmbedShadowElement.h:

LayoutTests:

Unskip existing iOS layout tests, update tests and expected results.

  • platform/ios-simulator/TestExpectations:
  • platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-expected.txt: Updated expected result based on the

changes to test youtube-flash-plugin-iframe.html.

  • platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width-expected.txt: Updated expected result

based on the changes to test youtube-flash-plugin-iframe-no-height-or-width.html.

  • platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width.html: Modified to check the

width of each embedded YouTube video to ensure that we respect it (if specified).

  • platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe.html: Substitute pseudo id -webkit-plugin-replacement

for -apple-youtube-shadow-iframe as the later was renamed to the former in <​https://trac.webkit.org/changeset/168442>.
Fix misspelling of the word "embed" in a comment.

Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r203521 r203522  
     12016-07-21  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION: Plugin replaced YouTube Flash videos always have the same width
     4        https://bugs.webkit.org/show_bug.cgi?id=159998
     5        <rdar://problem/27462285>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Unskip existing iOS layout tests, update tests and expected results.
     10
     11        * platform/ios-simulator/TestExpectations:
     12        * platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-expected.txt: Updated expected result based on the
     13        changes to test youtube-flash-plugin-iframe.html.
     14        * platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width-expected.txt: Updated expected result
     15        based on the changes to test youtube-flash-plugin-iframe-no-height-or-width.html.
     16        * platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width.html: Modified to check the
     17        width of each embedded YouTube video to ensure that we respect it (if specified).
     18        * platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe.html: Substitute pseudo id -webkit-plugin-replacement
     19        for -apple-youtube-shadow-iframe as the later was renamed to the former in <https://trac.webkit.org/changeset/168442>.
     20        Fix misspelling of the word "embed" in a comment.
     21
    1222016-07-21  Ryan Haddad  <ryanhaddad@apple.com>
    223

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r203388 r203522  
    25572557# iOS tests that assert:
    25582558platform/ios-simulator/ios/fast/text/combining-enclosing-keycap.html
    2559 platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width.html
    2560 platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe.html
    25612559
    25622560# Kerning, Ligatures, and Printer Fonts caused these tests to fail.

  • trunk/LayoutTests/platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-expected.txt

    r178197 r203522  
    1717PASS objectNoEmbed.tagName is "OBJECT"
    1818PASS document.querySelectorAll("iframe").length is 1
    19 PASS internals.shadowPseudoId(normalEmbedShadowRoot.firstChild) is "-apple-youtube-shadow-iframe"
     19PASS internals.shadowPseudoId(normalEmbedShadowRoot.firstChild) is "-webkit-plugin-replacement"
    2020PASS normalEmbedShadowRoot.firstChild.firstChild.tagName is "IFRAME"
    21 PASS internals.shadowPseudoId(objectEmbedShadowRoot.firstChild) is "-apple-youtube-shadow-iframe"
     21PASS internals.shadowPseudoId(objectEmbedShadowRoot.firstChild) is "-webkit-plugin-replacement"
    2222PASS objectEmbedShadowRoot.firstChild.firstChild.tagName is "IFRAME"
    23 PASS internals.shadowPseudoId(objectNoEmbedShadowRoot.firstChild) is "-apple-youtube-shadow-iframe"
     23PASS internals.shadowPseudoId(objectNoEmbedShadowRoot.firstChild) is "-webkit-plugin-replacement"
    2424PASS objectNoEmbedShadowRoot.firstChild.firstChild.tagName is "IFRAME"
    2525Normal Embed:

  • trunk/LayoutTests/platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width-expected.txt

    r178197 r203522  
    1313
    1414TEST COMPLETE
     15PASS getComputedStyle(embedNoHeight).width is "425px"
    1516PASS getComputedStyle(embedNoHeight).height is "150px"
    1617PASS getComputedStyle(embedNoWidth).width is "300px"
     18PASS getComputedStyle(embedNoWidth).height is "350px"
    1719PASS getComputedStyle(embedNoWidthHeight).width is "300px"
    1820PASS getComputedStyle(embedNoWidthHeight).height is "150px"
     21PASS getComputedStyle(objectNoHeight).width is "425px"
    1922PASS getComputedStyle(objectNoHeight).height is "150px"
    2023PASS getComputedStyle(objectNoWidth).width is "300px"
     24PASS getComputedStyle(objectNoWidth).height is "350px"
    2125PASS getComputedStyle(objectNoWidthHeight).width is "300px"
    2226PASS getComputedStyle(objectNoWidthHeight).height is "150px"

  • trunk/LayoutTests/platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe-no-height-or-width.html

    r178197 r203522  
    1717    setTimeout(function() {
    1818        embedNoHeight = document.getElementById('embed-no-height');
    19         shouldBe('getComputedStyle(embedNoHeight).height', '"150px"')
     19        shouldBe('getComputedStyle(embedNoHeight).width', '"425px"');
     20        shouldBe('getComputedStyle(embedNoHeight).height', '"150px"');
    2021
    2122        embedNoWidth = document.getElementById('embed-no-width');
    22         shouldBe('getComputedStyle(embedNoWidth).width', '"300px"')
     23        shouldBe('getComputedStyle(embedNoWidth).width', '"300px"');
     24        shouldBe('getComputedStyle(embedNoWidth).height', '"350px"');
    2325
    2426        embedNoWidthHeight = document.getElementById('embed-no-width-or-height');
    … …  
    2729
    2830        objectNoHeight = document.getElementById('object-no-height');
    29         shouldBe('getComputedStyle(objectNoHeight).height', '"150px"')
     31        shouldBe('getComputedStyle(objectNoHeight).width', '"425px"');
     32        shouldBe('getComputedStyle(objectNoHeight).height', '"150px"');
    3033
    3134        objectNoWidth = document.getElementById('object-no-width');
    32         shouldBe('getComputedStyle(objectNoWidth).width', '"300px"')
     35        shouldBe('getComputedStyle(objectNoWidth).width', '"300px"');
     36        shouldBe('getComputedStyle(objectNoWidth).height', '"350px"');
    3337
    3438        objectNoWidthHeight = document.getElementById('object-no-width-or-height');

  • trunk/LayoutTests/platform/ios-simulator/ios/plugin/youtube-flash-plugin-iframe.html

    r178197 r203522  
    2121        objectNoEmbed = document.getElementById('object-no-embed');
    2222
    23         // Test we don't change any embe/object tag to iframe.
     23        // Test we don't change any embed/object tag to iframe.
    2424        shouldBe('normalEmbed.tagName', '"EMBED"');
    2525        shouldBe('elinkEmbed.tagName', '"EMBED"');
    … …  
    3232        // Test we have the shadow root and the iframe player.
    3333        normalEmbedShadowRoot = internals.shadowRoot(normalEmbed);
    34         shouldBe('internals.shadowPseudoId(normalEmbedShadowRoot.firstChild)', '"-apple-youtube-shadow-iframe"');
     34        shouldBe('internals.shadowPseudoId(normalEmbedShadowRoot.firstChild)', '"-webkit-plugin-replacement"');
    3535        shouldBe('normalEmbedShadowRoot.firstChild.firstChild.tagName', '"IFRAME"');
    3636
    3737        objectEmbedShadowRoot = internals.shadowRoot(objectEmbed);
    38         shouldBe('internals.shadowPseudoId(objectEmbedShadowRoot.firstChild)', '"-apple-youtube-shadow-iframe"');
     38        shouldBe('internals.shadowPseudoId(objectEmbedShadowRoot.firstChild)', '"-webkit-plugin-replacement"');
    3939        shouldBe('objectEmbedShadowRoot.firstChild.firstChild.tagName', '"IFRAME"');
    4040
    4141        objectNoEmbedShadowRoot = internals.shadowRoot(objectNoEmbed);
    42         shouldBe('internals.shadowPseudoId(objectNoEmbedShadowRoot.firstChild)', '"-apple-youtube-shadow-iframe"');
     42        shouldBe('internals.shadowPseudoId(objectNoEmbedShadowRoot.firstChild)', '"-webkit-plugin-replacement"');
    4343        shouldBe('objectNoEmbedShadowRoot.firstChild.firstChild.tagName', '"IFRAME"');
    4444

  • trunk/Source/WebCore/ChangeLog

    r203520 r203522  
     12016-07-21  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION: Plugin replaced YouTube Flash videos always have the same width
     4        https://bugs.webkit.org/show_bug.cgi?id=159998
     5        <rdar://problem/27462285>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Fixes an issue where the width of a plugin replaced YouTube video loaded via an HTML embed
     10        element would always have the same width regardless of value of the width attribute.
     11
     12        For YouTube Flash videos the YouTube plugin replacement substitutes a shadow DOM subtree
     13        for the default renderer of an HTML embed element. The root of this shadow DOM subtree
     14        is an HTML div element. Currently we set inline styles on this <div> when it is instantiated.
     15        In particular, we set inline display and position to "inline-block" and "relative", respectively,
     16        and set an invalid height and width (we specify a font weight value instead of a CSS length value
     17        - this causes an ASSERT_NOT_REACHED() assertion failure in StyleBuilderConverter::convertLengthSizing()
     18        in a debug build). These styles never worked as intended and we ultimately created an inline
     19        renderer (ignoring display "inline-block") that had auto width and height. Instead it is sufficient
     20        to remove all these inline styles and create a RenderBlockFlow renderer for this <div> so that it
     21        renders as a block, non-replaced element to achieve the intended illusion that the <embed> is a
     22        single element.
     23
     24        * html/shadow/YouTubeEmbedShadowElement.cpp: Remove unused header HTMLEmbedElement.h and include
     25        header RenderBlockFlow.h. Also update copyright in license block.
     26        (WebCore::YouTubeEmbedShadowElement::YouTubeEmbedShadowElement): Remove inline styles as these
     27        never worked as intended.
     28        (WebCore::YouTubeEmbedShadowElement::createElementRenderer): Override; create a block-flow
     29        renderer for us so that we layout as a block, non-replaced element.
     30        * html/shadow/YouTubeEmbedShadowElement.h:
     31
    1322016-07-21  Myles C. Maxfield  <mmaxfield@apple.com>
    233

  • trunk/Source/WebCore/html/shadow/YouTubeEmbedShadowElement.cpp

    r183975 r203522  
    11/*
    2  * Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
     2 * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2727#include "YouTubeEmbedShadowElement.h"
    2828
    29 #include "HTMLEmbedElement.h"
     29#include "RenderBlockFlow.h"
    3030
    3131namespace WebCore {
    … …  
    4040{
    4141    setPseudo(AtomicString("-webkit-plugin-replacement", AtomicString::ConstructFromLiteral));
     42}
    4243
    43     setInlineStyleProperty(CSSPropertyDisplay, CSSValueInlineBlock);
    44     setInlineStyleProperty(CSSPropertyPosition, CSSValueRelative);
    45     setInlineStyleProperty(CSSPropertyWidth, CSSValue100);
    46     setInlineStyleProperty(CSSPropertyHeight, CSSValue100);
     44RenderPtr<RenderElement> YouTubeEmbedShadowElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
     45{
     46    return createRenderer<RenderBlockFlow>(*this, WTFMove(style));
    4747}
    4848

  • trunk/Source/WebCore/html/shadow/YouTubeEmbedShadowElement.h

    r183975 r203522  
    3636    static Ref<YouTubeEmbedShadowElement> create(Document&);
    3737
     38    RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
     39
    3840private:
    3941    YouTubeEmbedShadowElement(Document&);
Note: See TracChangeset for help on using the changeset viewer.