New issue
Advanced search Search tips

Issue 682441 link

Starred by 1 user
Status: Fixed
Owner:
mlamouri@chromium.org
Closed: Jan 2017
Cc:
rbyers@chromium.org
mlamouri@chromium.org
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug-Regression



Sign in to add a comment

screen orientation not propagated into iframes

Project Member Reported by yaoyingyu@google.com, Jan 18 2017 
Chrome Version: 55.0.2883.91
OS:Android Nougat

What steps will reproduce the problem?
(1) Open http://output.jsbin.com/toluze/1
(2) Rotate the phone 
(3) Observe that orientation does change.
(4) Open http://output.jsbin.com/gepawic, which embed previous page in an iframe
(5) Rotate the phone
(6) Orientation never changes. 

What is the expected result?
The screen orientation should change even if it's inside an iframe 

What happens instead?
It doesn't. The expected result can be seen on firefox mobile

Please use labels and text to provide additional information.

Judging from an old bug 
https://bugs.chromium.org/p/chromium/issues/detail?id=389249#c4
The screen orientation was working as expected sometime back in Sep 2014. Not exactly sure when the regression occurred.

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

Cc: peter@chromium.org rbyers@chromium.org mlamouri@chromium.org
Components: Blink>Layout
Thanks for the report.  Confirmed correct screen orientation is available only in the top document:

In devtools console with context at "top"

> window.screen.orientation
ScreenOrientation
  angle: 270
  onchange: null
  type: "landscape-secondary"
  __proto__: ScreenOrientation

In devtools console with context as "1"
> window.screen.orientation
ScreenOrientation
  angle: 0
  onchange: function ()
  type: "portrait-primary"
  __proto__: ScreenOrientation

I don't see an appropriate component for this - perhaps Layout-dev (since they own window.screen generally?).

+mlamouri and peter since they have some history.

I agree it sounds like this worked back in Sept 2014.  Is it possible to get a bisect on Android back potentially as far as about Chrome 35?

Comment 2 by e...@chromium.org, Jan 25 2017

Labels: Needs-Bisect

Comment 3 by e...@chromium.org, Jan 26 2017

Owner: mlamouri@chromium.org
Status: Assigned (was: Untriaged)

Comment 4 by mlamouri@chromium.org, Jan 27 2017

Status: Started (was: Assigned)
Fix on its way.
Project Member

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

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

commit 1c17214f719b1e715aa6c64848c87176c2ac90b0
Author: mlamouri <mlamouri@chromium.org>
Date: Fri Jan 27 23:45:35 2017

Send orientation change events to sub frames even if parent isn't active.

When a frame receive an orientatio change notification, it only carry
the notification over if it is active but the iframe might be listening
to the event and not the frame.

Unfortunately, the test checking this was subtly broken because it
initialised itself by looking for the current orientation, thus
initialising the top frame controller.

BUG= 682441 

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

[modify] https://crrev.com/1c17214f719b1e715aa6c64848c87176c2ac90b0/third_party/WebKit/LayoutTests/screen_orientation/orientationchange-event-subframe.html
[modify] https://crrev.com/1c17214f719b1e715aa6c64848c87176c2ac90b0/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp
[modify] https://crrev.com/1c17214f719b1e715aa6c64848c87176c2ac90b0/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h

Comment 6 by mlamouri@chromium.org, Jan 27 2017

Labels: -Needs-Bisect M-57 Merge-Request-57
The fix looks simply enough to be merged to M57.
Project Member

Comment 7 by sheriffbot@chromium.org, Jan 30 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 8 by bugdroid1@chromium.org, Jan 31 2017

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

commit 80b7f9b6005cc57cfb71797bb4edb8220a8717f7
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Tue Jan 31 02:37:55 2017

Send orientation change events to sub frames even if parent isn't active.

When a frame receive an orientatio change notification, it only carry
the notification over if it is active but the iframe might be listening
to the event and not the frame.

Unfortunately, the test checking this was subtly broken because it
initialised itself by looking for the current orientation, thus
initialising the top frame controller.

BUG= 682441 

Review-Url: https://codereview.chromium.org/2661663003
Cr-Commit-Position: refs/heads/master@{#446834}
(cherry picked from commit 1c17214f719b1e715aa6c64848c87176c2ac90b0)

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

[modify] https://crrev.com/80b7f9b6005cc57cfb71797bb4edb8220a8717f7/third_party/WebKit/LayoutTests/screen_orientation/orientationchange-event-subframe.html
[modify] https://crrev.com/80b7f9b6005cc57cfb71797bb4edb8220a8717f7/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.cpp
[modify] https://crrev.com/80b7f9b6005cc57cfb71797bb4edb8220a8717f7/third_party/WebKit/Source/modules/screen_orientation/ScreenOrientationControllerImpl.h

Comment 9 by mlamouri@chromium.org, Jan 31 2017

Status: Fixed (was: Started)
►

Sign in to add a comment