Intent to implement and ship: disallow navigations in the unload handler

447 показвания
Преминаване към първото непрочетено съобщение

Lucas Gadani

непрочетено,
5.05.2016 г., 15:50:235.05.16 г.
до blink-dev,Daniel Cheng

Contact emails

l...@chromium.org


Spec


Summary

Disallow navigations in the unload handler.


Interoperability and Compatibility Risk

When navigating away, Firefox ignores the navigation in the unload handler. Edge adds the navigation to the history, but continues to commit original navigation. Safari aborts the navigation. Chrome currently aborts the navigation the first time the frame is navigating, but commits the second time.


With the proposed change, blink would have the same behavior as Firefox, and be closer to Edge. Overall I think the risk is very low.


Ongoing technical constraints

None


Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes.


OWP launch tracking bug

https://crbug.com/590054


Requesting approval to ship?

Yes.

Charlie Reis

непрочетено,
5.05.2016 г., 16:28:525.05.16 г.
до Lucas Gadani,blink-dev,Daniel Cheng
I think it's worth clarifying Chrome's current behavior here, since I think only in-page navigations are allowed today.  Can you confirm if I'm correct about the cases below?

1) Cross-document navigations during unload.
I think these are always blocked in Chrome, whether cross-origin or not, right?

2) In-page navigations (fragments, pushState, etc) during unload.
I think Chrome allows these to interrupt the user's attempted navigation, as long as the attempted navigation is same-process.  (Your comment about them not working a second time doesn't apply if you assign the unload handler again.)  If the attempted navigation is cross-process, we may have some internal bugs.

FWIW, I'd love to see in-page navigations blocked as well, but I'm not sure if that will have an impact on pages that try to do a pushState or replaceState as the user leaves.

Charlie

Domenic Denicola

непрочетено,
5.05.2016 г., 16:31:265.05.16 г.
до Lucas Gadani,blink-dev,Daniel Cheng
From: Lucas Gadani [mailto:l...@chromium.org]

> The spec isn't clear on this issue (https://www.w3.org/TR/html5/browsers.html#unload-a-document).

This is the wrong section of the wrong spec. I think the spec is fairly clear that navigations should not be allowed during unload.

The relevant algorithm is https://html.spec.whatwg.org/multipage/browsers.html#navigate step 4.

Charlie Reis

непрочетено,
5.05.2016 г., 17:12:075.05.16 г.
до Domenic Denicola,Lucas Gadani,blink-dev,Daniel Cheng
Are we sure what Firefox's behavior is here?  It looks like they prevented navigations during unload in https://bugzilla.mozilla.org/show_bug.cgi?id=371360, but then ran into problems with sites that used unload handlers to prevent form submissions from ending up in history (see comment 53, comment 65, and https://bugzilla.mozilla.org/show_bug.cgi?id=409888).

From some manual tests, it does look like Firefox allows unload handlers to navigate to pages that are same origin with the attempted navigation.   Today, Chrome only allows this if the attempted navigation is same origin with the current page and the unload navigation is in-page.  I think that already breaks the form case that was mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=371360 (but that repro case is no longer online, so I can't verify).

I'd still love to see us prevent all navigations during unload if possible.  Maybe the form case is not a concern since we already don't support cross-document navigations during unload?

Charlie

TAMURA, Kent

непрочетено,
5.05.2016 г., 23:38:425.05.16 г.
до Lucas Gadani,blink-dev,Daniel Cheng
LGTM1.

--
TAMURA Kent
Software Engineer, Google


Lucas Gadani

непрочетено,
6.05.2016 г., 16:01:576.05.16 г.
до Charlie Reis,blink-dev,Daniel Cheng
On Thu, May 5, 2016 at 4:28 PM Charlie Reis <cr...@chromium.org> wrote:
I think it's worth clarifying Chrome's current behavior here, since I think only in-page navigations are allowed today.  Can you confirm if I'm correct about the cases below?

1) Cross-document navigations during unload.
I think these are always blocked in Chrome, whether cross-origin or not, right?

That's correct, the proposed change will only affect in-page navigations, since scheduled navigations are already blocked in Chrome.

 
2) In-page navigations (fragments, pushState, etc) during unload.
I think Chrome allows these to interrupt the user's attempted navigation, as long as the attempted navigation is same-process.  (Your comment about them not working a second time doesn't apply if you assign the unload handler again.)  If the attempted navigation is cross-process, we may have some internal bugs.

Currently, pushState does not interrupt the user's attempted navigation, it only adds the state to the history and continues with the navigation. Fragment navigations are the only ones that should be affected.

 

Lucas Gadani

непрочетено,
6.05.2016 г., 16:02:486.05.16 г.
до Domenic Denicola,blink-dev,Daniel Cheng
Thanks for the correction, I'm still not convinced that's what the spec says (and the behavior is different across browsers, but disallowing navigations is the behavior I think is most sensible.

Lucas Gadani

непрочетено,
6.05.2016 г., 16:10:196.05.16 г.
до Charlie Reis,Domenic Denicola,blink-dev,Daniel Cheng
Thanks for pointing this out, I've noticed that Firefox actually behaves differently if the navigation that triggers the unload handler is a cross-origin navigation or not. I've tested 8 different scenarios (javascript navigation, regular navigations, inpage navigations and pushState, between the same origin and cross origin). Here are the test cases if anyone wants to reproduce:


The only case where Firefox interrupts the navigation is for same-origin in-page navigations. Chrome interrupts for all in-page navigations, regardless if it's same origin or cross origin (however, when retrying the navigation Chrome will ignore the navigation in the unload handler). Firefox also ignores pushState in the unload handler, but Chrome adds the entry to the history.

Edge is an interesting case, where it'll never interrupt the navigation, but it will also execute navigations in the unload handler even for regular navigations (where all other browsers will block it).

Overall, I'd argue that blocking all navigations in the unload handler is the most sensible thing to do.

Charlie Reis

непрочетено,
6.05.2016 г., 16:52:056.05.16 г.
до Lucas Gadani,Domenic Denicola,blink-dev,Daniel Cheng
One clarification regarding same vs cross origin: there's actually 3 URLs involved: the current URL, the attempted navigation's URL, and the unload handler's URL.  Firefox doesn't care about the current URL-- I think it just checks whether the attempted navigation's URL and the unload handler's URL are same-origin.

Anyway, non-owner LGTM for blocking all navigations during unload.  Trying to do something like Firefox's behavior seems problematic in Chrome, since the attempted navigation is often cross-process and may have already committed by the time we run the unload handler in the old process.

Charlie

Rick Byers

непрочетено,
7.05.2016 г., 9:43:267.05.16 г.
до Charlie Reis,Daniel Cheng,Domenic Denicola,Lucas Gadani,blink-dev

Thanks for the detailed discussion.  Sounds low risk to me, but to help quantify it (and mitigate the risk some developer is actually relying on this) perhaps it makes sense to add a use counter with deprecation message for one milestone?  WDYT?

Lucas Gadani

непрочетено,
9.05.2016 г., 15:41:289.05.16 г.
до Rick Byers,Charlie Reis,Daniel Cheng,Domenic Denicola,blink-dev
That sounds reasonable, I'll work on a patch to add a counter and a message.

Lucas Gadani

непрочетено,
2.08.2016 г., 14:38:552.08.16 г.
до Rick Byers,Charlie Reis,Daniel Cheng,Domenic Denicola,blink-dev
Now that this has been in beta for a few weeks, chromestatus is showing usage to be <=0.0001%, I'd like to go ahead and ask approval to ship this on M54.

I'll keep monitoring the usage as it goes into stable, but unless there's a significant change I think it should be safe to ship this in M54.

Rick Byers

непрочетено,
2.08.2016 г., 14:50:152.08.16 г.
до Lucas Gadani,Charlie Reis,Daniel Cheng,Domenic Denicola,blink-dev
I double checked the internal M53-specific use counter values for UnloadHandler_Navigation and they're at 0.00001% of M53 PageVisits.  Note that since Android Beta is still M52, we don't really have any Android data yet, but I did a quick check of Android dev data and it's consistent with this very low usage.

So LGTM2 to remove (I consider tkent@'s LGTM1 to be still standing, since we have only strictly better information).

Chris Harrelson

непрочетено,
2.08.2016 г., 15:00:372.08.16 г.
до Rick Byers,Lucas Gadani,Charlie Reis,Daniel Cheng,Domenic Denicola,blink-dev
LGTM3

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

Joe Medley

непрочетено,
3.08.2016 г., 11:20:283.08.16 г.
до Lucas Gadani,Rick Byers,Chris Harrelson,Charlie Reis,Daniel Cheng,Domenic Denicola,blink-dev
Lucas,

Can you please create a status entry now.



Joe Medley | Technical Writer, Chrome DevRel | jme...@google.com | 816-678-7195
If an API's not documented it doesn't exist.

Joe Medley

непрочетено,
14.09.2016 г., 19:02:1614.09.16 г.
до blink-dev,dch...@chromium.org
Every cycle, Chrome DevRel (specifically, me) publishes a post on deprecations and removals like this one we did for Chrome 53. I was trying to add this feature to my draft for Chrome 54. I trying to make sure I understood it, I ran all test cases Lucas's zip file. My understanding of this is that in Chrome 54, all navigations in window.onunload are ignored.

Except that, I found this to be true in Chrome 52.

The only difference I observed the browser history.

Did I misunderstand? What am I missing? I could really use a push in the right direction here.

Thanks

Lucas Gadani

непрочетено,
14.09.2016 г., 22:14:3714.09.16 г.
до Joe Medley,blink-dev,dch...@chromium.org
That's right, for most things there should be no difference, since regular navigations were already blocked previously in Chrome. The only difference is that now we also block fragment navigations (i.e. navigating to window.location.hash = 'hashname' or navigating to #hashname).

You can also look at the layout test added that exercises this difference:

sexmo...@gmail.com

непрочетено,
22.09.2016 г., 4:45:1422.09.16 г.
до blink-dev,dch...@chromium.org
Отговор до всички
Отговор до автора
Препращане
0 нови съобщения