New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 655988 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

YouTube embed rewrite doesn't work if <object> has type attribute

Reported by lordd....@gmail.com, Oct 14 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.59 Safari/537.36

Example URL:
http://animemaga.ru/blog/makoto-sinkay-reklamshchik-ot-boga-8372

Steps to reproduce the problem:
If <object> has attribute type="application/x-shockwave-flash" Chrome doesn't rewrite YouTube legacy embed.

What is the expected behavior?
Chrome rewrites legacy Flash embed to HTML5 variant.

What went wrong?
Chrome displays legacy Flash embed.

Did this work before? No 

Is it a problem with Flash or HTML5? Both

Does this work in other browsers? Yes

Chrome version: 54.0.2840.59  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 23.0 r0

Minimal example:
<object data="http://www.youtube.com/v/HKRsFD_Spf8" type="application/x-shockwave-flash"></object>
These types of embeds are replaced in Firefox.
 
Cc: mlamouri@chromium.org
Components: Internals>Plugins>Flash>PreferHTML5 Blink>Media
Status: Untriaged (was: Unconfirmed)
On a first glance HTMLEmbedElement doesn't parse the data attribute (only src) and therefore doesn't apply the rewrite.
Owner: avayvod@chromium.org
Status: Started (was: Untriaged)
avayvod@ has a fix for this.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d5ee1a98ea763ab6231e71ffa03f55290a20c837

commit d5ee1a98ea763ab6231e71ffa03f55290a20c837
Author: avayvod <avayvod@chromium.org>
Date: Mon Oct 17 17:07:06 2016

Fix <object data=> YouTube Flash rewrites.

Some YouTube videos are embedded via <object data="http://www.youtube.com/v/AABBCCDD">.
Chrome currently only rewrites embeds that use <embed src="http://www.youtube.com/v/AABBCCDD">
The CL adds the logic for rewriting YouTube Flash plugin embed URLs with HTML5 embeds to the
HTMLObjectElement (this matches Firefox behavior).

BUG= 655988 
TEST=manual + updated browser tests

Review-Url: https://codereview.chromium.org/2416253002
Cr-Commit-Position: refs/heads/master@{#425704}

[modify] https://crrev.com/d5ee1a98ea763ab6231e71ffa03f55290a20c837/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/d5ee1a98ea763ab6231e71ffa03f55290a20c837/chrome/test/data/flash_embeds.html
[modify] https://crrev.com/d5ee1a98ea763ab6231e71ffa03f55290a20c837/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp

Labels: Merge-Request-55
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bf84ffa9ddc686758aeb218e2ed68c446dc944eb

commit bf84ffa9ddc686758aeb218e2ed68c446dc944eb
Author: joedow <joedow@chromium.org>
Date: Mon Oct 17 20:46:34 2016

Revert of Fix <object data=> YouTube Flash rewrites. (patchset #1 id:1 of https://codereview.chromium.org/2416253002/ )

Reason for revert:
This CL appears to be causing failures on ChromiumOS:
https://uberchromegw.corp.google.com/i/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/19066

browser_tests on Ubuntu-12.04 browser_tests on Ubuntu-12.04

678 disabled
10 flaky
failed 1

failures:
ChromeContentRendererClientBrowserTest.RewriteYouTubeFlashEmbed

Original issue's description:
> Fix <object data=> YouTube Flash rewrites.
>
> Some YouTube videos are embedded via <object data="http://www.youtube.com/v/AABBCCDD">.
> Chrome currently only rewrites embeds that use <embed src="http://www.youtube.com/v/AABBCCDD">
> The CL adds the logic for rewriting YouTube Flash plugin embed URLs with HTML5 embeds to the
> HTMLObjectElement (this matches Firefox behavior).
>
> BUG= 655988 
> TEST=manual + updated browser tests
>
> Committed: https://crrev.com/d5ee1a98ea763ab6231e71ffa03f55290a20c837
> Cr-Commit-Position: refs/heads/master@{#425704}

TBR=jochen@chromium.org,foolip@chromium.org,mlamouri@chromium.org,avayvod@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 655988 

Review-Url: https://codereview.chromium.org/2425623003
Cr-Commit-Position: refs/heads/master@{#425771}

[modify] https://crrev.com/bf84ffa9ddc686758aeb218e2ed68c446dc944eb/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/bf84ffa9ddc686758aeb218e2ed68c446dc944eb/chrome/test/data/flash_embeds.html
[modify] https://crrev.com/bf84ffa9ddc686758aeb218e2ed68c446dc944eb/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp

Comment 7 by dimu@chromium.org, Oct 18 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3efdec3c23c4fb82b88803d65daf114beae5553d

commit 3efdec3c23c4fb82b88803d65daf114beae5553d
Author: avayvod <avayvod@chromium.org>
Date: Wed Oct 19 16:43:36 2016

Fix <object data=> YouTube Flash rewrites.

Some YouTube videos are embedded via <object data="http://www.youtube.com/v/AABBCCDD">.
Chrome currently only rewrites embeds that use <embed src="http://www.youtube.com/v/AABBCCDD">
The CL adds the logic for rewriting YouTube Flash plugin embed URLs with HTML5 embeds to the
HTMLObjectElement (this matches Firefox behavior).

Trying to reland https://codereview.chromium.org/2416253002.
Split the browser test into 10 separate test cases trying to avoid OOM on Debug ChromiumOS builder.

BUG= 655988 
TEST=manual + updated browser tests

Review-Url: https://chromiumcodereview.appspot.com/2424293002
Cr-Commit-Position: refs/heads/master@{#426223}

[modify] https://crrev.com/3efdec3c23c4fb82b88803d65daf114beae5553d/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/3efdec3c23c4fb82b88803d65daf114beae5553d/chrome/test/data/flash_embeds.html
[modify] https://crrev.com/3efdec3c23c4fb82b88803d65daf114beae5553d/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 19 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7

commit 6fc8d4abccc5af8e14ebaf191544074f20c0e6c7
Author: Anton Vayvod <avayvod@google.com>
Date: Wed Oct 19 21:38:47 2016

Fix <object data=> YouTube Flash rewrites.

Some YouTube videos are embedded via <object data="http://www.youtube.com/v/AABBCCDD">.
Chrome currently only rewrites embeds that use <embed src="http://www.youtube.com/v/AABBCCDD">
The CL adds the logic for rewriting YouTube Flash plugin embed URLs with HTML5 embeds to the
HTMLObjectElement (this matches Firefox behavior).

Trying to reland https://codereview.chromium.org/2416253002.
Split the browser test into 10 separate test cases trying to avoid OOM on Debug ChromiumOS builder.

BUG= 655988 
TEST=manual + updated browser tests

Review-Url: https://chromiumcodereview.appspot.com/2424293002
Cr-Commit-Position: refs/heads/master@{#426223}
(cherry picked from commit 3efdec3c23c4fb82b88803d65daf114beae5553d)

Review URL: https://codereview.chromium.org/2434873003 .

Cr-Commit-Position: refs/branch-heads/2883@{#195}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/chrome/test/data/flash_embeds.html
[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp

Labels: M-55
Status: Fixed (was: Started)
Cc: ranjitkan@chromium.org
Labels: TE-Verified-M55 TE-Verified-55.0.2883.21
Rechecked this issue on Windows 7, Windows 10 , MAC 10.11.6 for chrome version 55.0.2883.21. Followed the below steps:

1) Installed chrome and navigated to the URL: http://animemaga.ru/blog/makoto-sinkay-reklamshchik-ot-boga-8372
2) Three videos displayed in the page, 2 require a sign in but was able to play the video without signing in.
3) Right clicked on the video player and observed that it was played using HTML5..

Note: The same video uses flash to play on chrome version 54.0.2840.59 Stable

Considering this issue to fixed and is working as intended on 55.0.2883.21. Adding TE-Verified labels.
Labels: TE-Verified-M56 TE-Verified-56.0.2897.0
Rechecked this issue on Windows 7, Windows 10 , MAC 10.11.6 for chrome version 56.0.2897.0. Followed the below steps:

1) Installed chrome and navigated to the URL: http://animemaga.ru/blog/makoto-sinkay-reklamshchik-ot-boga-8372
2) Three videos displayed in the page, 2 require a sign in but was able to play the video without signing in.
3) Right clicked on the video player and observed that it was played using HTML5..

Thanks.!
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7

commit 6fc8d4abccc5af8e14ebaf191544074f20c0e6c7
Author: Anton Vayvod <avayvod@google.com>
Date: Wed Oct 19 21:38:47 2016

Fix <object data=> YouTube Flash rewrites.

Some YouTube videos are embedded via <object data="http://www.youtube.com/v/AABBCCDD">.
Chrome currently only rewrites embeds that use <embed src="http://www.youtube.com/v/AABBCCDD">
The CL adds the logic for rewriting YouTube Flash plugin embed URLs with HTML5 embeds to the
HTMLObjectElement (this matches Firefox behavior).

Trying to reland https://codereview.chromium.org/2416253002.
Split the browser test into 10 separate test cases trying to avoid OOM on Debug ChromiumOS builder.

BUG= 655988 
TEST=manual + updated browser tests

Review-Url: https://chromiumcodereview.appspot.com/2424293002
Cr-Commit-Position: refs/heads/master@{#426223}
(cherry picked from commit 3efdec3c23c4fb82b88803d65daf114beae5553d)

Review URL: https://codereview.chromium.org/2434873003 .

Cr-Commit-Position: refs/branch-heads/2883@{#195}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/chrome/renderer/chrome_content_renderer_client_browsertest.cc
[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/chrome/test/data/flash_embeds.html
[modify] https://crrev.com/6fc8d4abccc5af8e14ebaf191544074f20c0e6c7/third_party/WebKit/Source/core/html/HTMLObjectElement.cpp

Comment 14 by dimu@google.com, Nov 4 2016

[Automated comment] removing mislabelled merge-merged-2840

Comment 15 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment