New issue
Advanced search Search tips

Issue 619036 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

img tags with id of "title" overrides document.title

Reported by amitchel...@gmail.com, Jun 10 2016

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36

Example URL:

Steps to reproduce the problem:
1. Create html page with <img id="title" src="" alt="" />
2. run page
3. console - window.document.title

What is the expected behavior?
not to cause a javascript error

What went wrong?
This breaks ALL js libraries that try to access document.title (this include microsoftajax.js)

Does it occur on multiple sites: N/A

Is it a problem with a plugin? No 

Did this work before? Yes before release of latest stable version (9th of June)

Does this work in other browsers? Yes 

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

This causes problems with partial postbacks, ajax calls, etc. Would also problems with any JS that requires document.title
 
testpage.html
32 bytes View Download
Forgot to add - this causes the javascript TypeError Illegal Invocation, and causes pages to not work correctly.

Comment 2 by tkent@chromium.org, Jun 10 2016

Labels: Needs-Bisect OS-Mac
Status: Untriaged (was: Unconfirmed)
Summary: img tags with id of "title" overrides document.title (was: img tags with id of "title")
Confirmed.

Repro step:
1. Load the following URL
    data:text/html,<title>PASS</title><img id="title" src="" alt="" /><script>alert(document.title)</script>

Expected: Alert dialog with "PASS" opens.
Actual: Nothing happens (Illegal Invocation on DevTools Console)

Cc: nyerramilli@chromium.org
Components: Blink
Labels: -Type-Compat -Needs-Bisect M-53 OS-Linux Type-Bug-Regression
Owner: yukishiino@chromium.org
Status: Assigned (was: Untriaged)
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/49cbb3034b92f09f510ae5e1bb9934ffbbd2631e..6737af735864f6477d52f5795d2ad1287a7a150f

suspecting https://chromium.googlesource.com/chromium/src/+/0fb38d20480525c24720b73aaca3845dba98082c
yukishiino @, Could you please check the above issue & help us in finding an owner it its not yours.

51.0.2670.0 - Good build
51.0.2671.0 - Bad build

Able to repro on Win7, Mac OSX 10.11.5, Ubuntu 14.04 using Chrome Dev 53.0.2763.0, Beta 52.0.2743.33, Stable 51.0.2704.84 and Canary 53.0.2766.0 using URL provided in c#2.

Comment 4 by tkent@chromium.org, Jun 13 2016

Components: -Blink Blink>Bindings
Labels: -Pri-2 -M-53 ReleaseBlock-Stable M-52 Pri-1
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 13 2016

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

commit cccf8630bac25057f6afd30febf8c428cd486b8c
Author: yukishiino <yukishiino@chromium.org>
Date: Mon Jun 13 15:52:34 2016

binding: Fixes the fallback mechanism of Document's named properties.

The old implementation only works for
a) there is a hidden prototype, and documentWrapper->GetPrototype()
  actually returns itself, or
b) fallbacked attributes are data-type properties.

Since we made attributes be accessor-type properties _and_ removed
the hidden prototype, the old implementation no longer works.

Fixed the issue using V8's GetRealNamedPropertyInPrototypeChain().

BUG= 619036 

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

[add] https://crrev.com/cccf8630bac25057f6afd30febf8c428cd486b8c/third_party/WebKit/LayoutTests/fast/dom/HTMLDocument/named-item-not-found.html
[modify] https://crrev.com/cccf8630bac25057f6afd30febf8c428cd486b8c/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp

Labels: Merge-Request-52 Merge-Request-51
Confirmed the fix on ToT.

Labels: TE-Verified TE-Verified-53.0.2767.0
this is working fine in #53.0.2767.0 on Win7, Mac OS X 10.11.5, Ubuntu 14.04.

adding TE - verified labels.

Comment 9 by tin...@google.com, Jun 14 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M51), manual review required.

Comment 10 by tin...@google.com, Jun 14 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)

Comment 11 by tin...@google.com, Jun 14 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M51), manual review required.
Before we approve merge to M51, Could you please confirm whether this change is well baked in Canary and safe to merge? Per comment #8 it is already verified in Canary but is it a safe merge?
Please note that we're cutting Stable RC soon for this week release. I will approve the merge based on reply for comment #12. 
Please merge the CL asap before 5PM so that it gets picked for the next beta promotion scheduled for 6/15. 
Cc: hablich@chromium.org
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 15 2016

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

commit cccf8630bac25057f6afd30febf8c428cd486b8c
Author: yukishiino <yukishiino@chromium.org>
Date: Mon Jun 13 15:52:34 2016

binding: Fixes the fallback mechanism of Document's named properties.

The old implementation only works for
a) there is a hidden prototype, and documentWrapper->GetPrototype()
  actually returns itself, or
b) fallbacked attributes are data-type properties.

Since we made attributes be accessor-type properties _and_ removed
the hidden prototype, the old implementation no longer works.

Fixed the issue using V8's GetRealNamedPropertyInPrototypeChain().

BUG= 619036 

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

[add] https://crrev.com/cccf8630bac25057f6afd30febf8c428cd486b8c/third_party/WebKit/LayoutTests/fast/dom/HTMLDocument/named-item-not-found.html
[modify] https://crrev.com/cccf8630bac25057f6afd30febf8c428cd486b8c/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp

yukishiino@, could you please take a look at comments #12 & #13 and reply asap. Thank you very much.
Sorry for the late response.  And thanks for testing with Canary.

I think the fix is quite safe.  The changed code is only used for look-up of named properties of |window.document| and no other places.  Thus, it's safe to merge.

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 15 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1173e2a3f72674e2f41fa7988c8b35f99741737

commit a1173e2a3f72674e2f41fa7988c8b35f99741737
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Jun 15 06:16:24 2016

binding: Fixes the fallback mechanism of Document's named properties.

The old implementation only works for
a) there is a hidden prototype, and documentWrapper->GetPrototype()
  actually returns itself, or
b) fallbacked attributes are data-type properties.

Since we made attributes be accessor-type properties _and_ removed
the hidden prototype, the old implementation no longer works.

Fixed the issue using V8's GetRealNamedPropertyInPrototypeChain().

BUG= 619036 

Review-Url: https://codereview.chromium.org/2065523002
Cr-Commit-Position: refs/heads/master@{#399461}
(cherry picked from commit cccf8630bac25057f6afd30febf8c428cd486b8c)

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

Cr-Commit-Position: refs/branch-heads/2743@{#363}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[add] https://crrev.com/a1173e2a3f72674e2f41fa7988c8b35f99741737/third_party/WebKit/LayoutTests/fast/dom/HTMLDocument/named-item-not-found.html
[modify] https://crrev.com/a1173e2a3f72674e2f41fa7988c8b35f99741737/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp

Approving merge to M51 branch 2704 based on comment #8 and comment #18. Please merge ASAP. Thank you.
Labels: -Merge-Review-51 Merge-Approved-51
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 15 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7de5e915f79aba616c2f349992c420246037a0ef

commit 7de5e915f79aba616c2f349992c420246037a0ef
Author: Yuki Shiino <yukishiino@chromium.org>
Date: Wed Jun 15 06:26:16 2016

binding: Fixes the fallback mechanism of Document's named properties.

The old implementation only works for
a) there is a hidden prototype, and documentWrapper->GetPrototype()
  actually returns itself, or
b) fallbacked attributes are data-type properties.

Since we made attributes be accessor-type properties _and_ removed
the hidden prototype, the old implementation no longer works.

Fixed the issue using V8's GetRealNamedPropertyInPrototypeChain().

BUG= 619036 

Review-Url: https://codereview.chromium.org/2065523002
Cr-Commit-Position: refs/heads/master@{#399461}
(cherry picked from commit cccf8630bac25057f6afd30febf8c428cd486b8c)

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

Cr-Commit-Position: refs/branch-heads/2704@{#722}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[add] https://crrev.com/7de5e915f79aba616c2f349992c420246037a0ef/third_party/WebKit/LayoutTests/fast/dom/HTMLDocument/named-item-not-found.html
[modify] https://crrev.com/7de5e915f79aba616c2f349992c420246037a0ef/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp

Status: Fixed (was: Started)
Thank you guys for your all efforts to land the fix.  As the fix has got merged to M51 and M52, I close this issue as Fixed.
Labels: -TE-Verified TE-Verified-M51 TE-Verified-M53 TE-Verified-51.0.2704.103
Tested the issue on Win7, Mac OS X 10.11.5, Ubuntu 14.04 using Stable # 51.0.2704.103, displaying 'Pass' alert message.

Adding TE-verified labels for stable.
Labels: TE-Verified-M52 TE-Verified-52.0.2743.49
Tested this on Win7, Mac OSX 10.11.5, Ubuntu 14.04 using Beta 52.0.2743.49 and used the URL given in c#2
 data:text/html,<title>PASS</title><img id="title" src="" alt="" /><script>alert(document.title)</script>

Alert dialog with "PASS" displayed, adding TE-Verified labels.

Sign in to add a comment