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.

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

Remove "abort these steps" wording #2878

Closed
domenic opened this Issue Aug 1, 2017 · 6 comments

Comments

3 participants
@domenic
Member

domenic commented Aug 1, 2017

In Infra, we have clarified that "return" or "throw" aborts the algorithm it is in. This convention was adopted after much of the prose in the HTML Standard was written, so HTML contains many now-redundant sequences like

then return null and abort these steps

or

then throw a TypeError and abort these steps.

These should remove the redundant "and abort these steps". Similarly, cases like

Otherwise, if the new value is the same as the body element, do nothing. Abort these steps.

should be converted to replace "Abort these steps" with "Return".

Doing so helps set a good example, and reduce confusion for people who read HTML (which is currently a mix of the new style and the old style).

There are currently 427 instances of "abort these steps". If any of them seem tricky, feel free to leave them in and we can take care of them later. Just getting the straightforward ones is a big enough task.

(This is part of #2053)

@sam0410

This comment has been minimized.

Show comment
Hide comment
@sam0410

sam0410 Oct 18, 2017

Contributor

Hi @domenic, I added a PR for this #3140. Please review.

Contributor

sam0410 commented Oct 18, 2017

Hi @domenic, I added a PR for this #3140. Please review.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Nov 14, 2017

Member

Progress! After 7ebc44a, many of these are gone. 71 remain, and some of them are legitimate, e.g. they occur inside "in parallel" steps (for which return/throw don't work).

But others should be replaced, e.g. the one in drawImage() or the ImageData constructor. (ImageData even has a "throw an ... and return"; we should remove the "and return".) So there's still some work to do here.

Member

domenic commented Nov 14, 2017

Progress! After 7ebc44a, many of these are gone. 71 remain, and some of them are legitimate, e.g. they occur inside "in parallel" steps (for which return/throw don't work).

But others should be replaced, e.g. the one in drawImage() or the ImageData constructor. (ImageData even has a "throw an ... and return"; we should remove the "and return".) So there's still some work to do here.

@shreyateeza

This comment has been minimized.

Show comment
Hide comment
@shreyateeza

shreyateeza Dec 18, 2017

Member

Hello! I would like to work on this issue. @domenic

Member

shreyateeza commented Dec 18, 2017

Hello! I would like to work on this issue. @domenic

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Dec 18, 2017

Member

Great! Check out https://github.com/whatwg/html/blob/master/CONTRIBUTING.md to get set up. Then, try to find all instances of "abort these steps" and replace them with "return". Some of them need to stay "abort these steps", but we can point those out when you get to them.

Member

domenic commented Dec 18, 2017

Great! Check out https://github.com/whatwg/html/blob/master/CONTRIBUTING.md to get set up. Then, try to find all instances of "abort these steps" and replace them with "return". Some of them need to stay "abort these steps", but we can point those out when you get to them.

@shreyateeza

This comment has been minimized.

Show comment
Hide comment
@shreyateeza

shreyateeza Dec 18, 2017

Member

@domenic Please assign me for this issue.

Member

shreyateeza commented Dec 18, 2017

@domenic Please assign me for this issue.

@domenic

This comment has been minimized.

Show comment
Hide comment
@domenic

domenic Dec 18, 2017

Member

@shreyateeza GitHub doesn't let me do that, but no fear, you've stated this issue is yours; nobody else will take it :). You can work on it without being assigned.

Member

domenic commented Dec 18, 2017

@shreyateeza GitHub doesn't let me do that, but no fear, you've stated this issue is yours; nobody else will take it :). You can work on it without being assigned.

shreyateeza added a commit to shreyateeza/html that referenced this issue Dec 19, 2017

shreyateeza added a commit to shreyateeza/html that referenced this issue Dec 20, 2017

shreyateeza added a commit to shreyateeza/html that referenced this issue Dec 22, 2017

shreyateeza added a commit to shreyateeza/html that referenced this issue Dec 22, 2017

shreyateeza added a commit to shreyateeza/html that referenced this issue Dec 22, 2017

domenic added a commit to shreyateeza/html that referenced this issue Jan 2, 2018

@annevk annevk closed this in #3303 Jan 9, 2018

annevk added a commit that referenced this issue Jan 9, 2018

Editorial: further cleanup "abort these steps"
This replaces "abort these steps" with "return" and removes redundant "abort these steps".

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