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

Issue 683385 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

password_controller.js relies on old non-standard DOM behavior

Project Member Reported by rbyers@chromium.org, Jan 20 2017

Issue description

password_controller.js has this code:

 __gCrWeb.fillPasswordFormWithData =
      function(formData, username, password, win, opt_normalizedOrigin) {
    var doc = win.document;

    // If unable to read the 'document' property from a frame in a different
    // origin, do nothing.
    if (!doc) {
      return false;
    }

But by spec, invoking win.document on a cross-origin iframe is supposed to throw a SecurityException, not return undefined.  This was fixed in WebKit 5 months ago: https://trac.webkit.org/changeset/205136

I'll put up a trivial patch to attempt to fix this.
 

Comment 1 by rbyers@chromium.org, Jan 20 2017

Cc: vabr@chromium.org

Comment 2 by rbyers@chromium.org, Jan 20 2017

Cc: melandory@chromium.org
Status: Started (was: Assigned)
https://codereview.chromium.org/2646733007/
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2017

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

commit e5222b703090cfb4e426db7c384a0fd47c980474
Author: rbyers <rbyers@chromium.org>
Date: Wed Jan 25 19:34:02 2017

Fix unhandled SecurityException in iOS password_controller

BUG= 683385 

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

[modify] https://crrev.com/e5222b703090cfb4e426db7c384a0fd47c980474/ios/chrome/browser/passwords/resources/password_controller.js

Comment 4 by dch...@gmail.com, Jan 25 2017

Thank you for fixing this.

Comment 5 by pkl@chromium.org, Jan 27 2017

Labels: -M-58 M-57
Cc: pinkerton@chromium.org cma...@chromium.org
Thank you for fixing this! 

Comment 7 by rbyers@chromium.org, Jan 27 2017

Labels: Merge-Request-57
Since this WebKit change is in iOS 10.3 beta, let's merge to M57 to ensure the fix is out in stable in time.
Project Member

Comment 8 by sheriffbot@chromium.org, Jan 27 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 27 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a849bed81fd7ca725791f96f74a48fdb0984a85f

commit a849bed81fd7ca725791f96f74a48fdb0984a85f
Author: Rick Byers <rbyers@chromium.org>
Date: Fri Jan 27 17:32:50 2017

Fix unhandled SecurityException in iOS password_controller

BUG= 683385 

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

Review-Url: https://codereview.chromium.org/2664493003 .
Cr-Commit-Position: refs/branch-heads/2987@{#149}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/a849bed81fd7ca725791f96f74a48fdb0984a85f/ios/chrome/browser/passwords/resources/password_controller.js

Status: Fixed (was: Started)
Cc: -vabr@chromium.org

Sign in to add a comment