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

Spec HTMLElement.outerText #668

Closed
cvrebert opened this issue Feb 10, 2016 · 15 comments · Fixed by #6653
Closed

Spec HTMLElement.outerText #668

cvrebert opened this issue Feb 10, 2016 · 15 comments · Fixed by #6653
Labels
compat Standard is not web compatible or proprietary feature needs standardizing

Comments

@cvrebert
Copy link
Member

(This should be feasible after #465 is resolved.)
Basic info: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/outerText
HTMLElement.outerText appears to be supported in all major browsers except Firefox, but currently doesn't seem to be spec'd anywhere.
Its getter seems to return the same value as innerText.
Its setter removes the element and replaces it with the given raw text string (no HTML parsing).

@Ms2ger
Copy link
Member

Ms2ger commented Feb 10, 2016

Is this necessary for compat?

@annevk
Copy link
Member

annevk commented Feb 10, 2016

Paging @miketaylr.

@cvrebert
Copy link
Member Author

So, any interest in this now that innerText got merged (#1678)? @miketaylr ?

@rniwa
Copy link

rniwa commented Oct 12, 2016

This is necessary for compat. All but Gecko supported this property for ages.

@annevk annevk added the compat Standard is not web compatible or proprietary feature needs standardizing label Oct 12, 2016
@Ms2ger
Copy link
Member

Ms2ger commented Oct 12, 2016

Do you have data to back that up? Gecko does not seem to ever have hit compat issues because of this.

@zcorpan
Copy link
Member

zcorpan commented Oct 17, 2016

Use counter currently around 0.1%

https://www.chromestatus.com/metrics/feature/timeline/popularity/214

httparchive

SELECT url, COUNT(*) AS num
FROM [httparchive:har.2016_09_15_chrome_requests_bodies]
WHERE body CONTAINS ".outerText"
GROUP BY url
ORDER BY num DESC

1317 rows; top 10 rows are

Row url num
1 http://adsearch.adkontekst.pl/_/both/?prefix=akon&namespace=qa_akon&nc=0&browser=safari 111
2 http://static.mtml.ru/js/constructor_head_scripts.js?1474033114 67
3 http://d3n8a8pro7vhmx.cloudfront.net/assets/tinymce-jquery-b16bb09336f0e7f04e5e1d9228b0b6e8.js 50
4 http://cdn.livestream.com/website/events/0.27.7/javascripts//player.js 43
5 http://static.mtml.ru/js/constructor_head_scripts.js?1473768899 31
6 https://assets.adobedtm.com/c4286b4b2b34cb9b097fac1cfe0e4ac48afd27e3/satelliteLib-de93594d0f2c832f2e24197005ac21ff61e4bca4.js 24
7 http://api.reftagger.com/v2/reftagger.js 23
8 http://api.reftagger.com/v2/RefTagger.js 22
9 http://static.mtml.ru/js/constructor_head_scripts.js?1474391042 20
10 https://cdn.tinymce.com/4/tinymce.min.js 18

Note that a url can have multiple pages.

Full results at
https://gist.github.com/zcorpan/5f3dc4c37367b8551e9295892b0aeb66

Would need further analysis to get a sense of the impact; some of these are dead code (fallback for textContent), but some could be affecting behavior, e.g. reftagger.js has this

if(e.options[o].outerText===(F.bibleVersion||"default").toUpperCase()){e.selectedIndex=o;break}

@zcorpan
Copy link
Member

zcorpan commented Oct 18, 2016

It seems to me it is a shorter path to interop if Gecko adds outerText than for everyone else to drop it. It's also a simple addition, as far as I can tell.

Looking through https://github.com/search?l=JavaScript&q=outertext+gecko&ref=searchresults&type=Code&utf8=✓ I mostly see polyfills of outerText or straight usage of it, rather than it being used for browser sniffing. So adding it for Gecko seems like it would be a small web compat win, and relatively low risk.

@Ms2ger do you object to adding this to Gecko?

@Ms2ger
Copy link
Member

Ms2ger commented Oct 18, 2016

I would prefer not to add, but I have no decision power for Gecko :)

@zcorpan
Copy link
Member

zcorpan commented Oct 19, 2016

@smaug---- WDYT?

@smaug----
Copy link

It's also a simple addition, as far as I can tell.

Is it? Do we know what outerText should do? If it is just a variant of the same algorithm as innerText, then adding it sounds ok, but I don't know how it has been implemented.

@foolip
Copy link
Member

foolip commented Oct 19, 2016

The outerText getter is an alias of innerText in Blink, but the setters actually don't share much code. The outerText setter looks a lot like outerHTML's though.

@domenic
Copy link
Member

domenic commented Oct 19, 2016

I wonder what the usage rates of the getter vs. setter are. Although even if the setter is low, I can't imagine 3 browsers will want to remove it.

@foolip
Copy link
Member

foolip commented Oct 19, 2016

If usage were minuscule and removing it would simplify matters it's not unthinkable, but I wonder if it would simplify anything. @zcorpan, what do you think this would look like in spec terms?

@zcorpan
Copy link
Member

zcorpan commented Oct 20, 2016

The getter would just be an alias of innerText.

The setter I thought would be like outerHTML's setter but instead of parsing HTML you construct a DocumentFragment using the same logic as in the innerText setter.

It turns out there are a few differences between Chromium and Edge 13 for outerText, and for outerHTML there's a difference between Edge 13 and IE 11.

  • Chromium does not allow setting outerText on br et. al., but Edge 13 and IE 11 do. This seems like a bug in Chromium (and the spec for innerText allows setting on any HTML element, same as innerHTML).
  • Chromium merges adjacent text nodes before/after the replaced element for outerText and outerHTML. This causes 3 mutation records in total. Edge 13 and IE 11 do the same for outerText, but this causes only 2 mutation records. Edge 13 (and Gecko) do not merge text nodes for outerHTML; 1 mutation record.

The spec for outerHTML does not say to merge text nodes. I don't see much point in doing that, and if Edge has already changed outerHTML to not do it, maybe could also change outerText to stop merging text nodes?

Tests
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4590
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4591
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/4592

@miketaylr
Copy link
Member

miketaylr commented Oct 20, 2016

Paging @miketaylr.
So, any interest in this now that innerText got merged (#1678)? @miketaylr ?

(whoops, these pings got lost in my inbox... 😞)

This is necessary for compat.

It would be cool to see actual broken pages. Looking at https://gist.github.com/zcorpan/5f3dc4c37367b8551e9295892b0aeb66 from @zcorpan I see a ton of (minified) TinyMCE usage.

But that function is only used in an IE codepath:

function toStringIE() {
  return dom.create('body', null, cloneContents()).outerText;
}

Makes me wonder about the other results.

All the dojo usage is in an IE codepath as well:

this.applyTextDir(node,has("ie")?node.outerText:node.textContent

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
Development

Successfully merging a pull request may close this issue.

9 participants