Iframe content no longer rendering
Reported by
seanrobe...@hypothes.is,
May 26 2017
|
|||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.0 Safari/537.36 Steps to reproduce the problem: 1. visit https://via.hypothes.is/https://stacks.tv 2. on the far right, you will see an arrow to open up a sidebar- click it to open the sidebar What is the expected behavior? You should see content inside the main box of the sidebar (if only a sign in block) What went wrong? No sidebar content visible Did this work before? Yes 59 Does this work in other browsers? Yes Chrome version: 60.0.3112.0 Channel: canary OS Version: OS X 10.11.6 Flash Version: The main contents are delivered via an iframe. After investigations, the DOM is still there, but the actual visual rendering is not happening. I tried toggling lots of settings to see if something would trigger it to render but nothing. Additionally, you can "reload frame" and it will render. At first this seemed like a loss of layer state do to some transitioning but that's not the issue it seems. Additionally, we have an extension (https://chrome.google.com/webstore/detail/hypothesis-web-pdf-annota/bjfhmglciegochdpefhhlphglcehbmek) that does the same embedding that does not have the same problem. The site mentioned in the reproduction steps is a proxy site that just injects our embedding code.
,
May 29 2017
,
May 29 2017
Able to reproduce the issue on the latest canary(61.0.3114.0) on Windows-10, Mac OS 10.12.4 and Linux Ubuntu 14.04. This is a regression issue broken in M-60. Last good build: 60.0.3083.0 First bad build: 60.0.3084.0 Changelog: =========== https://chromium.googlesource.com/chromium/src/+log/31ba8c6c5838fd6e65f3e341cfb5af0736186dc3..913ee5fe4cfcc42ebc568965169258bc37fb0237 Bisect script is pointing to https://codereview.chromium.org/2830713003 which looks to be Android specific and rotate-to-fullscreen flag related. However, bisected this twice on Windows-10 and Mac OS 10.12.4 and got the same suspect as pointed above. johnme@: Could the suspect be related to this as this also touched third_party/WebKit/Source/core/dom/ related files. Please re-assign if the change is unrelated. Thank you!
,
May 31 2017
My bad, thanks for the bug report and the bisect! This is because of https://github.com/WICG/IntersectionObserver/issues/164 / issue 646202 . I'll prepare a fix.
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/945f4bf50cac68aee482fa5165c3a43379ed2241 commit 945f4bf50cac68aee482fa5165c3a43379ed2241 Author: johnme <johnme@chromium.org> Date: Thu Jun 01 13:18:38 2017 [ElementVisibilityObserver] Fix regression with default threshold https://codereview.chromium.org/2830713003 (r468137, 913ee5fe4cfcc42ebc568965169258bc37fb0237) changed the default threshold of ElementVisibilityObserver from std::numeric_limits<float>::min() to zero, assuming that they would behave the same. It turns out that there's some confusion here - see https://github.com/WICG/IntersectionObserver/issues/164 - and for now we should continue to use std::numeric_limits<float>::min() to distinguish between completely hidden and partially visible. BUG= 726839 Review-Url: https://codereview.chromium.org/2919543002 Cr-Commit-Position: refs/heads/master@{#476265} [modify] https://crrev.com/945f4bf50cac68aee482fa5165c3a43379ed2241/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp [modify] https://crrev.com/945f4bf50cac68aee482fa5165c3a43379ed2241/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.h
,
Jun 1 2017
Requesting approval to merge 945f4bf50cac68aee482fa5165c3a43379ed2241 @{#476265} to m60. Small safe patch that just reverts to the previous behavior of ElementVisibilityObserver before the regression.
,
Jun 2 2017
Your change meets the bar and is auto-approved for M60. Please go ahead and merge the CL to branch 3112 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/848e7b224a1b681fe3154f3b5f6fe2054d285c5b commit 848e7b224a1b681fe3154f3b5f6fe2054d285c5b Author: John Mellor <johnme@chromium.org> Date: Fri Jun 02 17:03:35 2017 [ElementVisibilityObserver] Fix regression with default threshold https://codereview.chromium.org/2830713003 (r468137, 913ee5fe4cfcc42ebc568965169258bc37fb0237) changed the default threshold of ElementVisibilityObserver from std::numeric_limits<float>::min() to zero, assuming that they would behave the same. It turns out that there's some confusion here - see https://github.com/WICG/IntersectionObserver/issues/164 - and for now we should continue to use std::numeric_limits<float>::min() to distinguish between completely hidden and partially visible. BUG= 726839 Review-Url: https://codereview.chromium.org/2919543002 Cr-Original-Commit-Position: refs/heads/master@{#476265} Review-Url: https://codereview.chromium.org/2920863003 . Cr-Commit-Position: refs/branch-heads/3112@{#120} Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897} [modify] https://crrev.com/848e7b224a1b681fe3154f3b5f6fe2054d285c5b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp [modify] https://crrev.com/848e7b224a1b681fe3154f3b5f6fe2054d285c5b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.h
,
Jun 2 2017
,
Jun 6 2017
Tested the issue on Windows 7, Mac 10.12.4, Linux Ubuntu 14.04 using chrome version#60.0.3112.20 with the steps mentioned in comment #0 & 1.Observed that the user able to see the content inside the main box of the sidebar in the below URL. https://via.hypothes.is/https://stacks.tv Hence adding TE-Verified labels.Please find the attached screen cast for the same. Thanks!! |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by seanrobe...@hypothes.is
, May 26 2017