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

Issue 726913 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression
Team-Security-UX



Sign in to add a comment

Interstitial buttons are on bottom of page

Project Member Reported by f...@chromium.org, May 26 2017

Issue description

At certain desktop window sizes, the interstitial buttons are jumping to the bottom of the page. It looks like it's a problem with the logic around small-window sizing. 

The bug is in Edward's CL that landed in M60, and I don't want to revert it. Instead we should fix and then merge the fix to 60.
 
Screen Shot 2017-05-26 at 4.40.11 PM.png
222 KB View Download

Comment 1 by f...@chromium.org, May 26 2017

Components: UI>Browser>SafeBrowsing UI>Browser>Interstitials
Hmmm, I don't believe I changed the CSS in a way that affects the button positioning in my recent CL. 

It's more likely that this was something that I didn't pick up in this CL submitted a few weeks ago which simplified the media queries a lot: https://codereview.chromium.org/2788323002/

The media queries will need adjusting. There's a middling space that tablets and smaller desktop screens both occupy.

If someone wants to take a look before I'm back it's in: 

@media (max-width: 420px) and (min-height: 401px) and (min-width: 240px), (max-height: 736px) and (min-height: 240px) and (min-width: 421px)

body .nav-wrapper {} 



Comment 3 by f...@chromium.org, May 29 2017

Owner: ntfschr@chromium.org
I took a quick look on Friday and came to the same conclusion. I think we just need to adjust the media queries. Surprised no one has complained!

I'm OOO through June 6 & didn't have time to land a fix before Friday evening. Nate, could you please handle this? We should probably get a fix merged back quickly.
Was OOO on Friday, but I'll take a look today and see how we can adjust the media queries.
Pending CL is up at https://chromium-review.googlesource.com/c/518544/, just waiting for Edward Jung to be back for UI comments.

Comment 6 by vakh@chromium.org, Jun 2 2017

Labels: SafeBrowsing-Triaged
Thanks Nate, lgtm'd your CL. 
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 5 2017

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

commit a44afd2d732a3192329ea398ec030eab6cfe3aeb
Author: Nate Fischer <ntfschr@chromium.org>
Date: Mon Jun 05 20:55:03 2017

Choose desktop interstitial layout for medium views

We should choose the desktop layout over the mobile layout interstitials
for middle-sized views, since we're switching to the desktop layout too
soon.

This CL changes the threshold from height = 736px to 560px and removes
the extra margin on the details element. Together, these changes help
improve transitions between mobile and desktop:

 - We use the desktop layout for more middle view sizes, and we can
   display more of the details because we're wasting less space with
   padding
 - When we switch to mobile layout, the "Details" button is in nearly
   the same vertical spot as it would've been in desktop layout (it
   only moves 35px lower).
 - Clicking "details" in the desktop layout usually shows at least a
   tiny bit of the details on the screen. This serves as a visual cue
   that the user needs to scroll to see all the content. Otherwise, the
   "details" button looks as if it doesn't do anything.

Test: Manual - compared screenshots
Bug:  726913 
Change-Id: I326047767da1660eb5a9dc272551676819eaaa82
Reviewed-on: https://chromium-review.googlesource.com/518544
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477080}
[modify] https://crrev.com/a44afd2d732a3192329ea398ec030eab6cfe3aeb/components/security_interstitials/core/browser/resources/interstitial_v2.css
[modify] https://crrev.com/a44afd2d732a3192329ea398ec030eab6cfe3aeb/components/security_interstitials/core/browser/resources/interstitial_v2_mobile.js

Labels: Merge-Request-61
Labels: -Merge-Request-61 Merge-Request-60
Please tag with appropriate OSs.
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Technically affects all platforms, but due to screen dimensions this is only an issue for desktop platforms.
Project Member

Comment 13 by sheriffbot@chromium.org, Jun 6 2017

Labels: -Merge-Request-60 Hotlist-Merge-Approved Merge-Approved-60
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
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 6 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bcf1f9a352f42fb145a11c214439ff0567e28a8

commit 6bcf1f9a352f42fb145a11c214439ff0567e28a8
Author: Nate Fischer <ntfschr@chromium.org>
Date: Tue Jun 06 21:27:40 2017

Choose desktop interstitial layout for medium views

We should choose the desktop layout over the mobile layout interstitials
for middle-sized views, since we're switching to the desktop layout too
soon.

This CL changes the threshold from height = 736px to 560px and removes
the extra margin on the details element. Together, these changes help
improve transitions between mobile and desktop:

 - We use the desktop layout for more middle view sizes, and we can
   display more of the details because we're wasting less space with
   padding
 - When we switch to mobile layout, the "Details" button is in nearly
   the same vertical spot as it would've been in desktop layout (it
   only moves 35px lower).
 - Clicking "details" in the desktop layout usually shows at least a
   tiny bit of the details on the screen. This serves as a visual cue
   that the user needs to scroll to see all the content. Otherwise, the
   "details" button looks as if it doesn't do anything.

TBR=ntfschr@chromium.org

(cherry picked from commit a44afd2d732a3192329ea398ec030eab6cfe3aeb)

Test: Manual - compared screenshots
Bug:  726913 
Change-Id: I326047767da1660eb5a9dc272551676819eaaa82
Reviewed-on: https://chromium-review.googlesource.com/518544
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#477080}
Reviewed-on: https://chromium-review.googlesource.com/526477
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#211}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/6bcf1f9a352f42fb145a11c214439ff0567e28a8/components/security_interstitials/core/browser/resources/interstitial_v2.css
[modify] https://crrev.com/6bcf1f9a352f42fb145a11c214439ff0567e28a8/components/security_interstitials/core/browser/resources/interstitial_v2_mobile.js

Status: Fixed (was: Started)
Labels: TE-Verified-M60 TE-Verified-60.0.3112.24
Verified the issue on windows 10, Mac 10.12.5 and Ubuntu 14.04 using chrome beta version #60.0.3112.24 as per comment #0.

Observed that interstitial buttons are not seen at bottom of page after resizing browser window as seen in the screen shot in comment #0. Hence, the fix is working as expected.

Attaching screen cast and screen shot for reference.

Hence, adding the verified labels.

Thanks...!!
726913.jpeg
56.9 KB View Download
726913.mp4
3.8 MB View Download
Status: Verified (was: Fixed)
Bulk edit: marking stale 'fixed' bugs as 'verified' since they don't need verification at this point.

Sign in to add a comment