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

Issue 823173 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression
Team-Security-UX

Blocking:
issue 392354



Sign in to add a comment

HTTPS Error interstitial page blocks Select All context menu

Reported by larrylac...@yahoo.com, Mar 19 2018

Issue description

Example URL:
https://cacert.org         or badssl.com > Untrusted root

Steps to reproduce the problem:
1. Go to https://cacert.org
2. Get a 'Connection not private' warning with NET::ERR_CERT_AUTHORITY_INVALID
3. Tap the NET::ERR tag to display the diagnostic text
4. Long press anywhere in the diagnostic text

What is the expected behavior?
Get a context menu toolbar at the top with Copy, Select All..

What went wrong?
Long press not recognized

Did this work before? N/A 

Chrome version: 65.0.3325.109  Channel: stable
OS Version: 5.1.1
Flash Version: 

Tested on a Samsung tablet SM-T337A
with Chrome 63.0.3239.11 and 66.0.3359.0
and reported by other android phone users on 65.0.3325.109.

I believe this happens for all sites with a cert connection error.  The 2 sites named always fail.

On my tablet, on the Chrome tab, the busy-circle icon never stops cycling.  I believe it only goes in one direction.

On a Windows desktop, the busy-cycle ends after ~3 seconds, and the text is selectable.

As an Android works-OK reference point: for chrome://version, a long press opens the copy context menu bar at the top.

Without copy/paste of the connection error diagnostic text, it's difficult to troubleshoot user problems.
 
Labels: Needs-triage-Mobile
Cc: elawrence@chromium.org
Components: -Internals>Network UI>Browser>Interstitials
Summary: HTTPS Error interstitial page blocks Select All context menu (was: Connection Not Private page blocks Select All context menu)
Cc: nyerramilli@chromium.org pnangunoori@chromium.org sandeepkumars@chromium.org
Labels: -Type-Bug -Pri-2 FoundIn-66 RegressedIn-61 FoundIn-67 Target-67 hasbisect-per-revision Triaged-Mobile Target-65 FoundIn-65 Target-68 Pri-1 Type-Bug-Regression
Owner: amaralp@chromium.org
Status: Assigned (was: Unconfirmed)
Tested the issue in Android and able to reproduce the issue. 

Steps Followed:
1. Go to https://cacert.org
2. Get a 'Connection not private' warning with NET::ERR_CERT_AUTHORITY_INVALID
3. Tap the NET::ERR tag to display the diagnostic text
4. Long press anywhere in the diagnostic tex
5. Observe no context menu is seen.

Chrome versions tested:
65.0.3325.109 (Stable), 67.0.3372.0(Canary)

OS:
Android 8.1.0, 7.0.0

Android Devices:
Pixel XL, Samsung J7

Using the per-revision bisect providing the bisect results,
Good Build - 61.0.3117.0 
Bad Build - 61.0.3118.0  

You are looking for a change made after 476354(GOOD), but before 476355(BAD).

CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
https://chromium.googlesource.com/chromium/src/+/58d4d0b2d9511fe57bde3cbe82e486658183456c

From the CL above, assigning the issue to the owner concerned.

@amaralp: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.

Please navigate to below link for log's and video--
go/chrome-androidlogs/823173

Note: 
1. This issue is not observed in Desktop.
Note: If you test this on an 8" tablet, you can also visually observe that the active cycle icon on the tab, never finishes.
It looks like |InterstitialPageImpl| doesn't override |RenderViewHostDelegateView::ShowContextMenu()| so the menu selection menu isn't shown. Normally the selection menu is show through |WebContentsViewAndroid::ShowContextMenu()| which overrides that method in |RenderViewHostDelegateView|. I also noticed that dragging the selection handles does not work. Does anyone who works on interstitial know why we don't show context menus?
Cc: -elawrence@chromium.org carlosil@chromium.org
My guess is that context menus were disabled (or, never enabled) since with interstitials being non-committed navigations, the available actions on desktop (view source, print, etc.), would have not matched the interstitial, but the previous page.

Interstitials are currently being refactored into committed navigations, and with those the context menu works (since we no longer use InterstitialPageImpl) and actions would match the interstitial, but for the time being we could special case ShowContextMenu for Android, since the problematic elements don't show up there.
Cc: -pnangunoori@chromium.org elawrence@chromium.org
Also note, on large screen android, e.g. Samsung Tablet T8,  android 5.1.1, that display tabs: with the interstitial, the busy-cycle icon never completes (displayed in the tap). This 'busy' state may block dragging the handles.

Re: non-committed navigation, unless this gate only applies to applies to android, the logic for not displaying the context menu may be inconsistent with the Windows implementation which has a context menu (and completes the busy cycle).
I'll take a look at the spinner not completing for interstitials, regarding the context menu, it is disabled on Windows (and other platforms too). (you can try by going to expired.badssl.com and right clicking)
Labels: -Target-68 M-67 ReleaseBlock-Stable Target-66
Thanks for the investigation. 

It would good to have a fix for M67, tagging accordingly for tracking purpose. Feel free to modify if needed.
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 27 2018

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

commit a17de6fee41d662166d9a807ad33750491542e29
Author: Pedro Amaral <amaralp@chromium.org>
Date: Tue Mar 27 19:43:39 2018

Interstitial call ShowContextMenu on Android

Override |ShowContextMenu()| in InterstitialPageImpl and have it call
|WebContentsViewAndroid.ShowContextMenu()| on Android. This is done to
show the floating text selection menu (see linked bug).

Bug: 823173
Change-Id: I01090aee43536cd06c8f8bc28ebccf5c2767d0d8
Reviewed-on: https://chromium-review.googlesource.com/977240
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546210}
[modify] https://crrev.com/a17de6fee41d662166d9a807ad33750491542e29/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/a17de6fee41d662166d9a807ad33750491542e29/content/browser/frame_host/interstitial_page_impl.h

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Issue doesn't repro on latest M67, Now we are able to see test selection tool bar. But we do see some other issues like Copy, select all and device hardware back key is not Functional. we have separate bug "issue:828426" to track. 
Status: Available (was: Verified)
Turns out that my fix didn't actually help since the Copy and Select All options don't work (WebContentImpl::GetFocusedFrame() is returning nullptr). This makes me think that copying or selecting all on interstitial pages on Android never worked. I think the correct solution would be to enable context menus on interstitials on all platforms and that would fix Android. I'll revert my patch since showing the menu with broken options is worse than not showing it at all.
Blocking: 392354
Cc: -carlosil@chromium.org amaralp@chromium.org
Labels: -Pri-1 Pri-2
Owner: carlosil@chromium.org
IMO, it's probably not worth it to tackle this one by itself in that case, as it will be fixed with committed interstitials (which are currently targeted for 68). I will assign this to myself, but feel free to take it if this is needed before 68.
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 4 2018

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

commit f10cba2eafb0f3c661f2e3904eb420b01bafa60c
Author: Pedro Amaral <amaralp@chromium.org>
Date: Wed Apr 04 02:10:20 2018

Revert "Interstitial call ShowContextMenu on Android"

This reverts commit a17de6fee41d662166d9a807ad33750491542e29.

Reason for revert: The options in the selection menu don't work. Copy and Select All don't do anything.

Original change's description:
> Interstitial call ShowContextMenu on Android
> 
> Override |ShowContextMenu()| in InterstitialPageImpl and have it call
> |WebContentsViewAndroid.ShowContextMenu()| on Android. This is done to
> show the floating text selection menu (see linked bug).
> 
> Bug: 823173
> Change-Id: I01090aee43536cd06c8f8bc28ebccf5c2767d0d8
> Reviewed-on: https://chromium-review.googlesource.com/977240
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Carlos IL <carlosil@chromium.org>
> Commit-Queue: Pedro Amaral <amaralp@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#546210}

TBR=boliu@chromium.org,amaralp@chromium.org,carlosil@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 823173
Change-Id: Ia0743769b372b93acda2835c2274456f3a5379a0
Reviewed-on: https://chromium-review.googlesource.com/994191
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547953}
[modify] https://crrev.com/f10cba2eafb0f3c661f2e3904eb420b01bafa60c/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/f10cba2eafb0f3c661f2e3904eb420b01bafa60c/content/browser/frame_host/interstitial_page_impl.h

Labels: -ReleaseBlock-Stable -M-67 -Target-65 -Target-66 -Target-67 Target-68
Status: Assigned (was: Available)

Sign in to add a comment