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.

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

Make <select> special in innerText #1717

Merged
merged 2 commits into from
Nov 3, 2016
Merged

Make <select> special in innerText #1717

merged 2 commits into from
Nov 3, 2016

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Aug 26, 2016

select is a replaced element, but for Web compat we need
to include the text of option elements in innerText.
The new text is intended to match the behavior of Gecko,
which in turn is very close to the legacy IE/Edge behavior.

Fixes rocallahan/innerText-spec#5


@jfkthame are you in the spec's Acks? If not, which name do you prefer?

@zcorpan zcorpan added normative change compat Standard is not web compatible or proprietary feature needs standardizing labels Aug 26, 2016
@zcorpan
Copy link
Member Author

zcorpan commented Aug 26, 2016

Hmmm. I suppose optgroup and option should only be special-cased if they are in a select?

@zcorpan zcorpan mentioned this pull request Aug 26, 2016
14 tasks
@domenic
Copy link
Member

domenic commented Aug 26, 2016

Hmmm. I suppose optgroup and option should only be special-cased if they are in a select?

Probably? What do browsers do?

@domenic
Copy link
Member

domenic commented Sep 8, 2016

@zcorpan what's the plan here? Add acks, add tests, merge? Or back off this change since it only matches 1/4 browsers, apparently?

@zcorpan
Copy link
Member Author

zcorpan commented Sep 9, 2016

I think we can merge this and see if browsers align with this. There is a small benefit of this approach over the WebKit/Edge 14.14393 behavior in that it is closer to IE, where innerText has existed for the longest time, so maybe there is Web content expecting IE-like behavior. I should add more tests for this as well.

@domenic
Copy link
Member

domenic commented Sep 12, 2016

@zcorpan LGTM then. Can you add the acks? Will let you decide whether to merge before or after adding tests.

@zcorpan zcorpan added do not merge yet Pull request must not be merged per rationale in comment needs tests Moving the issue forward requires someone to write tests labels Oct 24, 2016
zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Nov 3, 2016
@zcorpan
Copy link
Member Author

zcorpan commented Nov 3, 2016

Tests at web-platform-tests/wpt#4166

@zcorpan zcorpan removed needs tests Moving the issue forward requires someone to write tests do not merge yet Pull request must not be merged per rationale in comment labels Nov 3, 2016
@zcorpan zcorpan removed their assignment Nov 3, 2016
domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 3, 2016
`select` is a replaced element, but for Web compat we need
to include the text of `option` elements in innerText.
The new text is intended to match the behavior of Gecko,
which in turn is very close to the legacy IE/Edge behavior.

Fixes rocallahan/innerText-spec#5
@domenic domenic merged commit 6aa98eb into master Nov 3, 2016
@domenic domenic deleted the innertext-select branch November 3, 2016 16:27
tomoyukilabs pushed a commit to tomoyukilabs/web-platform-tests that referenced this pull request Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat Standard is not web compatible or proprietary feature needs standardizing normative change
Development

Successfully merging this pull request may close these issues.

innerText should be supported for the <option> elements in a <select>
2 participants