Auto-dismissing JavaScript alerts

[Avi Drissman]

Good morning, platform-architecture-dev!

I'm writing you on behalf of the security team, and more precisely, as one of the security peeps who owns JavaScript alert dialogs and has to ratchet them down as web pages abuse them.

The biggest problem with JavaScript alert dialogs is that they are app-modal, and lock out all of Chrome until the user addresses them. That gives them an enormous power, a power that we want to limit.

The obvious solution here is to make alerts tab-modal, but there are nasty technical reasons as to why we can't do so. (I can elaborate if you want.) But Apple recently shipped Safari 9.1, which has a feature of auto-dismissing these dialogs. We were inspired by this feature and how it could solve our problem, so I wrote up a design doc:

https://docs.google.com/document/d/1wtV5rmLhbf1OZkbg7crtCt6h1mMtig_ctTQt3BLLEIU

Ojan recommended that I run this by you folks, to make sure that you weren't surprised, and to pick your brains if there's something here that we missed.

So thank you for your time, and I'm keen to know what you think.

Avi

[Ivan Kotenkov]

On Thursday, March 31, 2016 at 4:50:15 PM UTC+2, Avi Drissman wrote:

The obvious solution here is to make alerts tab-modal, but there are nasty technical reasons as to why we can't do so. (I can elaborate if you want.) 

Would you please elaborate? 

[Avi Drissman]

Certainly.

Optionally for alert, but always for confirm and prompt, the JavaScript engine is paused while waiting for the user's reply. The renderer is blocked in a synchronous call to the browser process, so all the tabs that are backed by that render process are also useless and not responsive.

If we allowed the user to switch to different tabs while the dialog was up, some tabs would not be responsive while others would be, which is a UI nightmare. The only UI solution that might work is if somehow we could make a dialog modal to the set of tabs that shared a render process. That has no consistency and is not easily explicable to the user.

Firefox solves this problem by running nested message loops, but that comes with other problems like out-of-order dialog closing, etc. See the section in the doc about that.

That's why I like this proposal (and what Apple has done with Safari 9.1); dismissing the dialog when you switch away means you can unlock all the tabs for interaction with the user.

See http://crbug.com/456 for a bit more elaboration on this synchronous blocking issue.

Avi

[Kentaro Hara]

I'm not eligible to say something from the perspective of UI and cross-browser compatibility, but from the perspective of renderer's architecture, your proposal totally makes sense.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CACWgwAaSZ21AETDn2hF991MaacYzxRUHxezXsTZQDJi5btmd5Q%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

[Ojan Vafai]

There were a couple open questions I want to be sure arch team is OK with:

1. Between queueing background dialogs vs showing a notification. I was very excited about showing a notification instead, but notifications are all moving to the system notifications UI. Once they do that, you won't be able to disable notifications for a specific site from the notification itself. Since these won't require a permission, there's a higher likelihood that we'll cause the user to disable notifications for all of Chrome. That's obviously bad. So, I think queueing background dialogs is the only reasonable option.

2. Should the code live in the embedder or inside Blink. I'm arguing that it should live inside Blink so that it can be tied to things we expose to web authors. Specifically, if the visibilityState=="hidden" or the top-level window is blurred, then your notification will be non-blocking. If visibilityState=="visible" and the top-level window is focused, then your notification will be blocking.

Do those both seem reasonable to you?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jyw5PrsmFSa-9AZNStbTTEePBW7LBw11GF4c_heW35irg%40mail.gmail.com.

[Avi Drissman]

The problem that I have with 2 is that rather than a contained change that doesn't affect the content api and is easily implementable today, that approach would involve massive plumbing changes. Also, you speak about the "top-level window being blurred". Is that something that the web authors can see? I don't see how we can do this entirely using web-visible attributes.

Avi

[Ojan Vafai]

On Tue, Apr 12, 2016 at 8:45 PM Avi Drissman <a...@chromium.org> wrote:

The problem that I have with 2 is that rather than a contained change that doesn't affect the content api and is easily implementable today, that approach would involve massive plumbing changes.

I was picturing that this all happens on the blink side of the content boundary and that content doesn't need to know anything about this. But, I haven't looked at the code in question in a long while. Is that unreasonable for some reason? Ultimately, the calls all originate in Blink.

 

Also, you speak about the "top-level window being blurred". Is that something that the web authors can see? I don't see how we can do this entirely using web-visible attributes.

I thought you could use window blur/focus events, but now that I look more closely they're not quite right. They can see when the top-level window is blurred/focused via blur/focus events. It does get blurred when a tab is backgrounded or when a different window gets focus. So, that matches your use case.

There's a couple catches that make this a bit less nice than is ideal:

1. You can only access the top-level window if you're same origin. In practice, I think this is fine since we're actually going to try to disable modal dialogs from cross-origin frames entirely.

2. Focusing a sub-frame also causes the top-level window to blur. So you're actual logic needs to be whether any frame on your page has focus and I think it doesn't work at all for cross-origin frames since you can't tell when they're focused.

3. Focusing the omnibox causes the top-level window to blur. This is the one that makes me feel window blur/focus isn't good enough.

We might want consider in extending visibilitychange to have a new value for the case where a tab is foreground, but in an unfocused window.+Ilya Grigorik wdyt?

We need *some* way for web authors to reason about what's going to happen when they use these APIs.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CACWgwAYjsW-Uj-267DsFmbUfFZH8HT692yfmFEuh7b%3D7K6y9OQ%40mail.gmail.com.

[Avi Drissman]

On Thu, Apr 14, 2016 at 12:49 AM, Ojan Vafai <oj...@chromium.org> wrote:

I was picturing that this all happens on the blink side of the content boundary and that content doesn't need to know anything about this. But, I haven't looked at the code in question in a long while. Is that unreasonable for some reason? Ultimately, the calls all originate in Blink.

Doing this at the embedder level simplifies things greatly, because that means that we can implement it one platform at a time. Otherwise this becomes a huge discussion that we'll have to have with every embedder.

 

We might want consider in extending visibilitychange to have a new value for the case where a tab is foreground, but in an unfocused window.+Ilya Grigorik wdyt?

And this is where I start worrying. We're trying to reduce usage of these ancient APIs that are way too powerful by reducing their power, and you're suggesting that to do so we start adding new stuff to the web platform, just for them? :(

 

We need *some* way for web authors to reason about what's going to happen when they use these APIs.

But the web authors have better solutions: don't do that. They have notifications. They're easy to sub in for alert, and for confirm they can be used for the focus stealing, alerting the user that a decision needs to be made when they switch back.

I see us as having two choices. 1. Do this at the embedder level. It's significantly simpler and easier to implement, but has the downside that perhaps the web authors have less visibility into what's going on, or 2. Do this in Blink, which significantly complicates things as it affects every embedder out there, but has the upside that we can give web authors more visibility as to what's going on... for an obsolete API that we're already trying to discourage, and only if we add more stuff to the web platform to support us depowering this obsolete API.

Avi 

[Ojan Vafai]

On Mon, Apr 18, 2016 at 8:58 AM Avi Drissman <a...@chromium.org> wrote:

On Thu, Apr 14, 2016 at 12:49 AM, Ojan Vafai <oj...@chromium.org> wrote:

I was picturing that this all happens on the blink side of the content boundary and that content doesn't need to know anything about this. But, I haven't looked at the code in question in a long while. Is that unreasonable for some reason? Ultimately, the calls all originate in Blink.

Doing this at the embedder level simplifies things greatly, because that means that we can implement it one platform at a time. Otherwise this becomes a huge discussion that we'll have to have with every embedder.

You've sold me that we can do this at the embedder level (see below). 

For the sake of codebase simplicity, I think we want to do this for all embedders eventually unless there is specific pushback. Who do we contact for this? The embedders I know of are Opera and Chromium Embedded Framework. Are there others? Can we ask them if they care?

I suspect other browser vendors to follow suite once we ship this, so it will effectively become part of the web platform enshrined in specs. I don't expect any embedders would want to be left behind and have web content half-work for them.

  

We might want consider in extending visibilitychange to have a new value for the case where a tab is foreground, but in an unfocused window.+Ilya Grigorik wdyt?

And this is where I start worrying. We're trying to reduce usage of these ancient APIs that are way too powerful by reducing their power, and you're suggesting that to do so we start adding new stuff to the web platform, just for them? :(

 

We need *some* way for web authors to reason about what's going to happen when they use these APIs.

But the web authors have better solutions: don't do that. They have notifications. They're easy to sub in for alert, and for confirm they can be used for the focus stealing, alerting the user that a decision needs to be made when they switch back.

You're totally right. I was thinking about this incorrectly. If you care about the behavior here, we already have the better alternative (notifications). I think that's good enough developer optics.

I see us as having two choices. 1. Do this at the embedder level. It's significantly simpler and easier to implement, but has the downside that perhaps the web authors have less visibility into what's going on, or 2. Do this in Blink, which significantly complicates things as it affects every embedder out there, but has the upside that we can give web authors more visibility as to what's going on... for an obsolete API that we're already trying to discourage, and only if we add more stuff to the web platform to support us depowering this obsolete API.

I'm sold.

 

Avi 

--
You received this message because you are subscribed to the Google Groups "Chrome Intervention Team" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chrome-interventio...@google.com.
To post to this group, send email to chrome-inter...@google.com.
To view this discussion on the web visit https://groups.google.com/a/google.com/d/msgid/chrome-intervention-team/CACWgwAZ2jvo9yRR_OhcH5cYVv4knBfiEwpLhCMJh%3DtGfwwx9Cg%40mail.gmail.com.

[Avi Drissman]

On Mon, Apr 18, 2016 at 2:11 PM, Ojan Vafai <oj...@chromium.org> wrote:

For the sake of codebase simplicity, I think we want to do this for all embedders eventually unless there is specific pushback. Who do we contact for this? The embedders I know of are Opera and Chromium Embedded Framework. Are there others? Can we ask them if they care?

Opera and CEF are the big ones. I've already run this by Philip Jägenstedt, who's commented on the doc, but he didn't comment on the "Opera" section which is good news. :) I haven't yet run this by Marshall; I'm going to do that right now.

Every time I've done any significant change to this area, it's never been an issue with the embedders, but with Android WebView. Any change we make has to allow it to continue to comply with the API contracts that it has made.

 

I'm sold.

Excellent.

As a side note, I'm planning on logging in the devtools console when we do anything fancy. Even though this will be embedder-side, I want to make ensure that the answer to "wait, what just happened" is still "look at the console".

Avi

[Ojan Vafai]

On Mon, Apr 18, 2016 at 11:23 AM Avi Drissman <a...@chromium.org> wrote:

On Mon, Apr 18, 2016 at 2:11 PM, Ojan Vafai <oj...@chromium.org> wrote:

For the sake of codebase simplicity, I think we want to do this for all embedders eventually unless there is specific pushback. Who do we contact for this? The embedders I know of are Opera and Chromium Embedded Framework. Are there others? Can we ask them if they care?

Opera and CEF are the big ones. I've already run this by Philip Jägenstedt, who's commented on the doc, but he didn't comment on the "Opera" section which is good news. :) I haven't yet run this by Marshall; I'm going to do that right now.

Every time I've done any significant change to this area, it's never been an issue with the embedders, but with Android WebView. Any change we make has to allow it to continue to comply with the API contracts that it has made.

+Alexandre Elias can you comment on whether we can change this for Android WebView?

  

I'm sold.

Excellent.

As a side note, I'm planning on logging in the devtools console when we do anything fancy. Even though this will be embedder-side, I want to make ensure that the answer to "wait, what just happened" is still "look at the console".

Makes sense to me. Thanks!

 

Avi

[Alexandre Elias]

I'd propose Android WebView change nothing and punt the responsibility to embedders.  Given the existing API, if a third-party web browser built on top of Android WebView wants to implement OldSpice behavior, it's within their power and responsibility to do so using the existing Java APIs exposed today.

The contract is that the onJsAlert Java method (or the variants below) should be called on the UI thread when the JS calls alert().  The Blink thread should block until the application Java code calls the onJsResult callback.  In practice, most embedders don't implement it, in which case no UI is shown and the JS immediately receives a return value of false (if there were two options).

  

For part A "foremost tab exit" behavior, WebView has zero control over what kind of UI the embedding app shows so there is nothing to do there.  For part B hiding alerts on background tabs, the concept of backgrounded "tab" is meaningless from the perspective of WebView, but there is the option to make a change when the entire app is backgrounded.  In that case, I think it's a bad idea to try to batch them within WebView and call onJsAlert when the app returns to foreground -- there's a risk of null pointer crashes and such.  Something more sane is to silently drop the alerts -- this would also violate the contract, although I'm guessing no apps would actually break from that -- but it would leave WebView with a crummy compromise UX and deny the possibility for third-party browsers to *actually* reproduce the proper OldSpice UX if they wanted.  So my conclusion is that punting to embedder is best.