Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Should the spec require the page-specified onbeforeunload message not be used? #952

Closed
avidrissman opened this issue Mar 28, 2016 · 1 comment
Closed

Should the spec require the page-specified onbeforeunload message not be used? #952

avidrissman opened this issue Mar 28, 2016 · 1 comment
Assignees
@domenic
Labels
removal/deprecation Removing or deprecating a feature

Comments

@avidrissman
Copy link

In its onbeforeunload function, a page specifies specify a string to be shown in the prompt. According to the spec in step 7:

The prompt shown by the user agent may include the string of the returnValue attribute, optionally truncated.

The majority of browsers take advantage of the word "may" in that step and do not include the page's string in that prompt:

  • Firefox does not do so, and hasn't since Firefox 4. As they say in the relevant bug, they removed that ability as it was a security issue.
  • Safari does not do so starting with version 9.1. They too consider this a security fix for the vulnerability that was assigned CVE-2009-2197 and noted in their release notes.
  • Chrome and Opera have stopped doing so as of Chrome 51 and Opera 38. As with the other vendors, we consider using page-supplied text in a privileged-appearing dialog as a security fix.

Is it appropriate for the spec to follow the behavior of the browsers here?
@domenic
Copy link
Member

domenic commented Mar 29, 2016

I agree that we should update the spec to not process the string at all. If the majority of browsers agree that omitting the string is a security fix, the spec should not suggest an insecure path. I can work on a patch for this next week (although PRs are welcome in the meantime).

/cc @travisleithead @DigiTec as an FYI, since this would make Edge have non-standard behavior.

@domenic domenic added the removal/deprecation Removing or deprecating a feature label Mar 29, 2016
@domenic domenic self-assigned this Mar 29, 2016
@domenic domenic mentioned this issue Apr 5, 2016
Merged
domenic added a commit that referenced this issue Apr 5, 2016
@domenic
Ignore beforeunload event return value
4163263 
Instead of using it as the message to show to the user, simply compare
it against the empty string or not. This matches Gecko, WebKit, and
Blink (but not yet EdgeHTML).

Closes #952.
domenic added a commit that referenced this issue Apr 6, 2016
@domenic
Ignore beforeunload event return value
1ffc09d 
Instead of using it as the message to show to the user, simply compare
it against the empty string or not. This matches Gecko, WebKit, and
Blink (but not yet EdgeHTML).

Closes #952.
domenic added a commit that referenced this issue Apr 8, 2016
@domenic
Ignore beforeunload event return value
01d9870 
Instead of using it as the message to show to the user, simply compare
it against the empty string or not. This matches Gecko, WebKit, and
Blink (but not yet EdgeHTML).

Closes #952.
jungkees pushed a commit to jungkees/html that referenced this issue Apr 8, 2016
@domenic @jungkees
Ignore beforeunload event return value
5c3bf16 
Instead of using it as the message to show to the user, simply compare
it against the empty string or not. This matches Gecko, WebKit, and
Blink (but not yet EdgeHTML).

Closes whatwg#952.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@domenic domenic

Labels
removal/deprecation Removing or deprecating a feature
Milestone
No milestone
Development

No branches or pull requests
2 participants
@domenic @avidrissman