I did some digging into this today, and here's what I found.

- Tapping anywhere above or below the "Show original" text in the info bar does not register the tap. Given the vertical size of the info bar compared to the vertical size of the text, it is very easy to miss the tap target (in my experience). I am not sure what logic the web contents uses to match taps to links, but this does not feel like the same logic here.

- Interestingly, the tap target for "Show original" appears to extend all the way from the end of the "Show original" text to the right of the info bar, until the 'X' (close) button. This seems wrong as well since I would not expect tapping to the right of the text to follow the link.

So to summarize, the target is wider, but shorter than expected? The former seems fine to me, but not the latter.

Were you able to reproduce the behavior mdw@ observed? I was able to only when I decreased the font size, but the tap always registered if I tried several times. I was using a Nexus 6p running Android N.

Yes, I've been able to reproduce this by decreasing font size also. Turning on pointer location shows if the center of the click isn't touching the link, then the reload doesn't happen and the infobar isn't dismissed. The infobars use the SizeToPreferredSize() for links (see url below). 

Reassigning this to pkasting@ to handle if we should add padding to the link view in infobars.

https://cs.chromium.org/chromium/src/chrome/browser/ui/views/infobars/infobar_view.cc?rcl=1485274695&l=117

It's not clear to me how to take action on your request.  Are you asking a UI design question for Android specifically, or some sort of technical question about how touch event targeting should work on Android, or what?

Note that I don't contribute to Android and the Android infobar system is a bit different than on desktop, so I'm almost certainly not the right assignee regardless.  But a first step would be making it clearer what "if we should add padding to the link view" means; restate the problem (as it's not clear how your solution is related to the first paragraph of comment 4), give a detailed solution proposal, describe how it affects non-Android platforms, etc.

@pkasting 

We use the Chrome infobar, not the Android infobar to avoid several unnecessary jni calls. So while this is an infobar shown on Android, it is entirely created using the Chrome/components infobar implementation and doesn't touch any Android code.

The issue is that the Chrome infobars that are created using the code linked above are displayed on Android, and the link clicked event isn't being called when expected because the user has to be particularly accurate with no space to click above or below the link text.

This relates to the first paragraph because mdw@ thought an action was happening without any indication when he clicked the link, including the infobar not being dismissed, when our "show original" link was clicked. The underlying issue is not due to slow loading as was expected, but due to the the UI itself. 

I looped you in as the owner, because I haven't worked in the Chrome UI implementation. I thought as the owner of both the chrome ui and components infobar you would have input as to how to improve the infobar when a user expects it to be clicked, but it isn't being due to the view area being tight vertically around the link. Have you worked with anyone else on the infobars that could address this problem?

At least on Views, I thought we handled touch events via rect-based targeting, to avoid problems where center point-based targeting did not hit the view in question.  It seems like perhaps that would solve this.  But I'm not familiar with how Android handles events and decides what to target.

I would think something like rect-based targeting would be what you'd want, not hacking around this with extra padding around the link, but I'm not sure.  +CC tdanderson who worked on rect-based targeting on views long ago, but really this seems like an Android-specific UI implementation technical question for whomever does event dispatch?

Ahh makes sense. Since I've only created UI elements solely in Android before, all I knew is that the link view wasn't getting the touch event, but haven't looked at the targeting of those being dispatched to Chrome UI.

Yes, Peter is correct in that we do use a "rect-based targeting" heuristic in native UI code (src/ui/views). If you're interested in seeing the details of how it works, take a look at ViewTargeterDelegate::TargetForRect() in ui/views/view_targeter_delegate.cc.

The heuristic is only enabled for Win, Linux, and CrOS since when it was implemented those were the only platforms using ui/views. In a local build, can you force IsRectBasedTargetingEnabled() to return true (so that the code in RootViewTargeter::FindTargetForGestureEvent() to set the bounding box is reached), re-test the UI, and let me know if you see any change in the touch accuracy?

For debugging views, you may find this useful: https://www.chromium.org/developers/how-tos/inspecting-ash though I'm not 100% sure it will work.

Side question: apart from the UI we're talking about here, do you know of any other places in Chrome for Android that uses the views toolkit?

Neither mdw@ nor I could tell any difference when forcing IsRectBasedTargetingEnabled() true. Is there anything that could be preventing this from working on Android?

I don't know of any other places that Chrome for Android uses the the views toolkit, but there's a lot of clank that I don't know. 

I'm not at all familiar with how event-processing works in Chrome on Android, and until seeing this bug I was under the impression Views was not used at all. I think as a next step you should trace through the code to figure out the logic used to select the target for the given input event.

Good places to start, assuming that gesture targeting is working the way I anticipate, would be to place logging in the following places to see that they are in fact being reached:

* RootViewTargeter::FindTargetForGestureEvent(), specifically near the code that is behind IsRectBasedTargetingEnabled().
* ViewTargeterDelegate::TargetForRect().
* RootView::OnEventProcessingStarted().
* EventProcessor::OnEventFromSource() is the main event processing logic that I assume is being used.

You should also check that the event type is in fact a gesture event; if it's a raw touch event, then Views will never receive it.

If this UI is indeed an InfoBarView as you say in #4, your best bet is to try getting a stack trace from within InfoBarView::DoesIntersectRect(); this method should be getting called during event-targeting, and you should be able to trace back to the logic that is being used. It's also worth double-checking that InfoBarView::ButtonPressed() is getting called.

Please let me know what you can find out.

Drive-by comment here: Maybe using a link is the wrong way to implement this, and instead it should be button (or something else). I don't think we're wedded to the link UI, though we'd need to review that with UX folks of course. Right now, though, the implementation is broken and I want to avoid rolling this out to more users until it's fixed.

aposner -- I think we need to make this issue blocking any launch of the new UI.

I think if the only way we're able to reproduce the issue by reducing the size of text, this shouldn't be launch blocking.

Separately, I know a button UI on the infobar has come up previously. Cc'ing Bruno here in case he knows more. 

The look of the UI and its implementation should be separate. I agree with Ariel is the only situation in which this happens is by reducing the size of the text, I don't think it's blocking.

And if the issue caused by the component which draws the link, perhaps it should be reworked to have the same tap area as an equivalent button, but with the current look.

Looks like it is an Android issue after digging around some. Android uses a span to set the clickable link text in the main text, and I'm not sure how easy it will be to change the span implementation without affecting other infobars. It's not simply a tap area that we can increase the size of. I'm going to look into if we can easily increase the click area of this by toying with how it's implemented by not putting it in the same text block as the main text, but otherwise it maybe something that Android just determines. Should I even bother with this work, or are we wanting a button fix over the link fix? 

Note that this issue is *not* related to font or display size -- it happens regardless of the font or display aize (though, a smaller font size probably makes it easier to accidentally miss the tap target).

I personally would prefer a button as I think it's odd for the infobar to show this pseudo "link" (it's not a link). Bruno, what say you?

I'm not a fan of the link either, as it's not navigational, but an action (the attachment would at least technically be more correct, from a UX perspective).
I don't think this blocks the shipment of the Data Saver infobars to which this is connected, and I'm not sure exactly who'd handle this change, as it's an infobar pattern, even if a new one.

Deleted: infobar.png 4.8 KB

	infobar.png 4.8 KB View Download

Megan, how hard is it to implement the UI from #17?

+dfalcantara for infob spec changes

Using a clickablespan does indeed give us very little control for determining the bounding region of the click area.

The best we can do is to split the text into two components.  One is the button one is the text.  It limits the ability for the text to be "in the flow" of the other sentence, and they really need to be separate concepts.

Megan, any progress on this?

Nope, no progress. Dan and Ted, how do we feel about splitting the two components? I can start work on that. Are we using it elsewhere "in the flow" that would cause this to be an issue?

Which components are you mentioning, specifically?  Like, moving the link to be a button so that you can actually click it?  That's a change to the infobar spec, which doesn't currently account for that.  You'd probably have to adjust this one:

https://folio.googleplex.com/chrome-ux-specs-and-sources/Clank%20-%20Material%20design/Preview%20and%20Specs/_specs#%2FSPECS-Infobar-03%20Control-less%20InfoBar.png%3Fz=width

As noted earlier, this also doesn't fix the other places where infobars have clickable text links, which we can't adjust the bounds for.  They probably couldn't be adjusted the same way, anyway; a lot of those are "Learn more" links that would result in a third button on the infobar.

Ariel, we need figure out how we want to work with the spec on this. Do we want to adjust to the spec mentioned in comment 23? Do we want to try to change the spec?

Just to clarify, these are the options (please correct me if I'm mistaken):

1. Change the clickable link into a different component, which would improve the bounding of the click area, but wouldn't keep it in the flow of the text.
2. Change the infobar spec to include the action from #17

My understanding is that we won't have room for #17 and a timestamp in the infobar - Bruno, let me know if this is incorrect.

Before we decide to pursue #1, what does it mean for the link to not be in the flow of the text? Does it mean it would have to go on its own line?

You're correct.

I also don't understand why we wouldn't be able to grow the tappable area of the link without moving it to a separate row. I understand there would be work involved, but would it be impossible to keep it in the flow?

The problem is that we don't own the link tap logic at all for links within text.  That is handled in the bowels of TextView in Android.  Determining whether a click is over a link area would require knowing the layout of all the links, which would require knowing how all the font glyphs are laid out.  In general, this is stuff we don't know about.  Getting this information would be very, very expensive and likely wrong.

Basically, we have little to no visibility when a click happened within a text view and it is entirely within their code to figure it out.

That makes sense. I guess the actual next step would be to look at all instances of links in infobars and come up with a UX proposal to replace them, right?

Pretty much, yeah.  LMK if you need help finding those.  I was just working on cleaning up the code that added all of these links to these infobars.

I'm stepping in for a bit to help with this. dfalcantara - as far as I can tell all the info bars that this applies to are already listed here in this spreadsheet:
https://docs.google.com/spreadsheets/d/1BQpxtgIDr-KoZMS-cQ8H3D0TpQwzkcJBvROhevMqxe0/edit#gid=1292301464

ANDROID_DOWNLOAD_MANAGER_OVERWRITE_INFOBAR_DELEGATE
GENERATED_PASSWORD_SAVED_INFOBAR_DELEGATE_ANDROID
SAVE_PASSWORD_INFOBAR_DELEGATE
TRANSLATE_INFOBAR_DELEGATE

Are there any new ones/changes? I think there's at least a Page Preview one we should add.

I there's also one for "SubresourceFilterExperimentalInfoBar" that was just added, and one for the search geolocation one was added around december.

+ktam who was interested in reading up on this

Reassigning to hannahs@ as primary contact (thanks) but this may be something another designer ends up resolving.

Seems the best choice is to a temporary fix for the preview info bar and then a longer-term one that includes a minimal info bar with a button. Sent an email to UI Review posing this for discussion:
https://groups.google.com/a/google.com/forum/#!topic/chrome-ui-review/Ph-8akk0ZjE

And I believe some UX people + ainslie@ are meeting on Tuesday to discuss this further.

hannahs@: any update? thanks.

Hey guys, seems like this was resolved off thread, Dan was there another bug created to track this? Can't seem to find it! >__<

Sending out a friendly ping

I can't seem to find any bugs for this either.  +mdjones@ in case he saw any bugs fly by during the great infobar bug migration

I think the problem will always remain as long as we allow links inline with the main text of an infobar.

Even putting the links at the end of the infobar text won't work nicely past a single line of main text.  If the main text is longer than a single line, we'd end up needing to put the links on a separate line to give them more clickable space.

The current behavior is the equivalent of <span>s in html, but to give larger click regions, they'd need to convert to <div>s where they are no longer inline rendered but block.

One possible solution could be to make the entire text block clickable - with the caveat that each text block in an infobar have only one text link (the ToS and Privacy links in payments infobar is the only 2 link instance I know of and can possibly be combined as one link that has both disclaimers on the same page.)

Can this issue be closed?

Ping. Is there any updates here, or can we close this?

Burgan: FYI as we update Previews UX.

I believe this is being worked on from a UX perspective (moving away from infobar for previews).

+dougarnett, is there anything concrete here that would let us update this bug more meaningfully?

Since the current plan is to move away from infobar (per bshealy@), it may not make sense to do something (eg, whole area clickable or button) for this infobar in the interim. I think this is a Won'tFix.

Pinging this thread as this bug is still relevant for capping heavy pages and downloads infobars. Is there any progress on either the interim solution (One possible solution could be to make the entire text block clickable - with the caveat that each text block in an infobar have only one text link) or a long term solution?

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

commit 35fab120be70aa7d57f588e7adecddf58917b2fb
Author: Ted Choc <tedchoc@google.com>
Date: Mon Jun 25 19:39:20 2018

Allow full infobar text to be clickable if it contains a single span.

On Android, the InfoBar text layout is handled by Android, which
causes the click region fo the links to be very small.  For InfoBar
messages with a single clickable span, this enables the full text to
be clickable.

BUG= 670340 

Change-Id: I1cc2dd0ffd34a583ae2040859b003c33c2e12baf
Reviewed-on: https://chromium-review.googlesource.com/1114002
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570136}
[modify] https://crrev.com/35fab120be70aa7d57f588e7adecddf58917b2fb/chrome/android/java/res/layout/infobar_control_message.xml
[modify] https://crrev.com/35fab120be70aa7d57f588e7adecddf58917b2fb/chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarCompactLayout.java
[add] https://crrev.com/35fab120be70aa7d57f588e7adecddf58917b2fb/chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarMessageView.java
[modify] https://crrev.com/35fab120be70aa7d57f588e7adecddf58917b2fb/chrome/android/java_sources.gni

Thanks, Ted!