New issue
Advanced search Search tips

Issue 682282 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocked on:
issue 682258



Sign in to add a comment

Implement <iframe allowfullscreen> in terms of feature policy

Project Member Reported by iclell...@chromium.org, Jan 18 2017

Issue description

The "allowfullscreen" iframe attribute should be implemented direcly in terms of Feature Policy, rather than the current implementation, which is an mashup of two different techniques: crawling the frame tree to the root, looking for either a chain of 'allowfullscreen' frames or an active policy.

If present on the iframe tag, allowfullscreen should translate into a declared policy

{ "fullscreen": ["*"] }

for the purpose of calculating the inherited policy for the nested frame.

 

Comment 1 by e...@chromium.org, Jan 18 2017

Cc: foolip@chromium.org
Labels: -Type-Bug -Pri-2 Pri-3 Type-Feature
Status: Available (was: Untriaged)
As a prerequisite to shipping this, I'm going to add a UseCounter to be triggered whenever the allowfullscreen flag changes after the frame is attached to the document.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 8 2017

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

commit 2906e0c1298431ea3cdb9fb3a8da8046d7c9e6bb
Author: iclelland <iclelland@chromium.org>
Date: Wed Feb 08 06:50:30 2017

Add UseCounter to see how often the "allowfullscreen" attribute is modified.

Before implementing snapshotting of the iframe allowfullscreen attribute, we
should have some idea of how often the existing live attribute is being modified
after the frame is attached to a document. This adds a new UseCounter named
HTMLIFrameElementAllowfullscreenAttribute to track this.

BUG= 682282 

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

[modify] https://crrev.com/2906e0c1298431ea3cdb9fb3a8da8046d7c9e6bb/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/2906e0c1298431ea3cdb9fb3a8da8046d7c9e6bb/third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp
[modify] https://crrev.com/2906e0c1298431ea3cdb9fb3a8da8046d7c9e6bb/tools/metrics/histograms/histograms.xml

Labels: Merge-Request-57
I'd like to merge this use counter into M57, so that we can get collect data about the safety of changing the snapshot behaviour in M58.

Adding merge-request-57 label
Project Member

Comment 5 by sheriffbot@chromium.org, Feb 9 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
Please merge your change to M57 branch 2987 before 5:00 PM PT, Friday 02/10 so we can take it in for next week beta release. Thank you.
Labels: -Merge-Approved-57 merge-merged-2987
While I'm waiting for bugdroid to notice this, I can report that this was merged into branch 2987 as commit b32edb1046f3a28615c9366ab78ca2ec1599d45c (https://crrev.com/b32edb1046f3a28615c9366ab78ca2ec1599d45c)

Updating appropriate labels manually.

Comment 8 by lunalu@chromium.org, May 26 2017

Ping. Didn't we do this already?
Cc: iclell...@chromium.org
Owner: lunalu@chromium.org
Status: Started (was: Available)
Yes (although there is still one complication around same-origin <frame> tags that needs to be resolved)


https://crrev.com/44b442a4f245e3fbfa17478bf0807cc139ab39ef implemented this.
Owner: loonyb...@chromium.org
Status: Fixed (was: Started)
Same-origin issue is resolved ( issue 729658 ), so closign as fixed.

Sign in to add a comment