New issue
Advanced search Search tips

Issue 716542 link

Starred by 2 users
Status: Verified
Owner:
ntfschr@chromium.org
Closed: Apr 2017
Cc:
fdoray@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 707481



Sign in to add a comment

safe_browsing_blocking_page_test fails for mobile layout

Project Member Reported by ntfschr@chromium.org, Apr 28 2017 
The tests in safe_browsing_blocking_page_test.cc succeed for the Desktop layout, but will fail for the mobile layout.

The test checks the visibility of elements with a GetVisibility() helper method, but this only checks for offsetHeight and offsetWidth. This works for desktop, because we hide things with `display: none` (which is inherited), but breaks on mobile layout because we use opacity instead (not inherited).

Checking visibility of elements must be done by recursively checking the visibility of parent elements.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 28 2017 

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

commit 3d74f72c743bef9d6fb48d06b7638fdf0cb7c5aa
Author: ntfschr <ntfschr@chromium.org>
Date: Fri Apr 28 20:26:03 2017

Properly check the visibility of elements in browsertests

No change in production logic.

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG= 716542 

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

[modify] https://crrev.com/3d74f72c743bef9d6fb48d06b7638fdf0cb7c5aa/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc

Comment 2 by ntfschr@chromium.org, Apr 28 2017

Status: Fixed (was: Assigned)

Comment 3 by fdoray@chromium.org, May 2 2017

Cc: fdoray@chromium.org
 Issue 716406  has been merged into this issue.

Comment 4 by ntfschr@chromium.org, Aug 24

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