User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0 Build ID: 20160630070928 Steps to reproduce: Every combo box in the vodafone easybox interface shows up as "undefined". Depending on what setting you alter, it either breaks making changes or they succeed without error. Chrome and edge both don't exhibit this behavior. mozregression points this between these commits: 11:07.42 INFO: Last good revision: 798c82c8a96d1099cf712434d9c9036fc48d8297 11:07.42 INFO: First bad revision: 9160f08660b8290559e427fd80d617edd86fe2a6

Any page to reproduce the issue?

Here's pushlog for the range, there is only bug 264412. https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=798c82c8a96d1099cf712434d9c9036fc48d8297&tochange=9160f08660b8290559e427fd80d617edd86fe2a6 The patch added element.innerText, so that could be related (perhaps, script uses in the page detects the existence of innerText, and uses different code path for each case?), but without testcase it's hard to figure out the issue. then, looks like "the vodafone easybox" is a DSL router or modem, so it's hard to expect the interface is publicly accessible.

Ah, another broken router UI, like in bug 1268833 for Cisco. Jan, can you save the broken router page and attach it to the bug reprot. Be sure by loading it locally you have all the CSS to reproduce the issue.

Unfortunately, it does use js to do pretty much everything. So no easy test case producing. I've poked a little with FireBug and so far this is what it seems to do: First it creates/loads the HTML to show using placeholder text. Example stuff it creates is: <span class="sText">STR_Password_account_security</span> or <option value="5" class="sText">STR_Password_5_min</option> At some point the function "replaceString" is called, see below for a copy. It walks over all items of class "sText", then evals (!) their text and replace the text with the results. The text is always some STR_* stuff which are variables declared based on language choice. This seems to break for <option> tags. Specifically, after executing this line over the <option> with STR_Password_5_min strToSplit = (strToReplace[i].innerText).split(separator); strToSplit is empty. If I indeed inspect that <option> in DOM its innerText is empty, while innerHTML is "STR_Password_5_min" as expected. For non-<option> tags, innerText is the same as innerHTML. Afterwards, it evals strToSplit, leading to "Undefined" as the final result. var separator = ','; // separator function replaceString() { var strToReplace = $('.sText').toArray(); var hasInnerText = (strToReplace[0].innerText != undefined) ? true : false; var tmpString = ''; var strToSplit, i, j; var ptrn_html = /<(.|\n)*?>/g; for (i = 0; i < strToReplace.length; i++) { strToSplit = new Array(); if (!hasInnerText) { strToSplit = (strToReplace[i].textContent).split(separator); tmpString = eval(strToSplit[0]); for (j = 1; j < strToSplit.length; j++) { try { tmpString = tmpString.replace('%s', eval(strToSplit[j])); } catch (err) { tmpString = tmpString.replace('%s', strToSplit[j]); } } if(ptrn_html.test(tmpString)) strToReplace[i].innerHTML = tmpString; else strToReplace[i].textContent = tmpString; } else { strToSplit = (strToReplace[i].innerText).split(separator); tmpString = eval(strToSplit[0]); for (j = 1; j < strToSplit.length; j++) { try { tmpString = tmpString.replace('%s', eval(strToSplit[j])); } catch (err) { tmpString = tmpString.replace('%s', strToSplit[j]); } } if(ptrn_html.test(tmpString)) strToReplace[i].innerHTML = tmpString; else strToReplace[i].innerText = tmpString; } } replaceButton(); }

I found the case that innerText keeps empty while innerHTML has value. not sure if this is the exact same case tho, this should be at least related. > <select> > <option id="a"></option> > </select> > > <script type="text/javascript"> > > var x = document.getElementById("a"); > x.innerHTML = "foo"; > console.log("innerText", x.innerText); > console.log("innerHTML", x.innerHTML); > > </script> this happens when setting innerHTML (or innerText) of option element inside select. this doesn't happen when there is no enclosing select element.

Based on the testcase, mark this as confirmed. This works as expected in Blink and Edge.

Hi Jonathon, Since you are working on another innerText bug, do you mind taking a look here? Thanks!

AFAICS, this should fix the behavior here: I think we want textframes corresponding to <option> elements to return their text content as .innerText, even though they're descendants of an nsListControlFrame that is of type eReplaced.

Comment on attachment 8779489 [details] [diff] [review] Make innerText return text from <option> elements within a <select>, rather than ignoring them <select>foo</select> and <select><option>foo</option></select> in blink gives "" for innerText on select element. > for (nsIFrame* f = aFrame->GetParent(); f; f = f->GetParent()) { > if (f->IsFrameOfType(nsIFrame::eReplaced) && >- !f->GetContent()->IsHTMLElement(nsGkAtoms::button)) { >+ !f->GetContent()->IsHTMLElement(nsGkAtoms::button) && >+ !f->GetContent()->IsHTMLElement(nsGkAtoms::select)) { > return false; > } But this makes <select><option>foo</option></select> case select.innerText return "foo" I think we have no good reason to not follow blink here.

Hmm, so blink returns empty for the select.innerText? That seems... odd. I wonder whether the web depends on that behavior? Or is it spec'd anywhere? I'd have naively expected that an element's innerText would normally include the concatenation of its (visible) children's innerText.

FWIW, another case where browsers differ is <option id="a">foo</option> (i.e. _without_ an enclosing <select>). Here, gecko returns "foo" for #a.innerText, while blink returns empty.

roc's innertText draft is the only reasonable spec-ish thing for innerText. https://github.com/rocallahan/innerText-spec

(In reply to Jonathan Kew (:jfkthame) from comment #11) > FWIW, another case where browsers differ is > > <option id="a">foo</option> > > (i.e. _without_ an enclosing <select>). Here, gecko returns "foo" for > #a.innerText, while blink returns empty. Hmm, that is interesting, since <option> can be used outside <select> - inside <datalist>

(In reply to Olli Pettay [:smaug] from comment #12) > roc's innertText draft is the only reasonable spec-ish thing for innerText. > https://github.com/rocallahan/innerText-spec According to roc's draft, AFAICS, our current behavior here is correct, and blink's is wrong. But clearly there are pages out there that depend on blink's behavior and are broken with ours, hence this bug. So probably the spec(-ish thing) should be modified to reflect this. I still think blink's behavior on <select><option>foo</option></select> where select.innerText returns empty is highly counter-intuitive, however, and wonder if we should press for a change there, if it won't break existing content. They currently have the situation where getting innerText for one element returns its (expected) text, yet moving up the DOM to an ancestor node and getting innerText will return LESS text than the child returned. One other observation: loading roc's innerText-getter tests[1] in Chrome, I get "120 passes, 67 fails", while Firefox Nightly gives me "193 passes, 0 fails". Given this, do we really want to converge towards Chrome's behavior? If so, it looks like the (draft) spec and tests could do with a substantial overhaul. [1] http://rocallahan.github.io/innerText-spec/getter-tests.html

it is probably somewhere in middle ground. I'm sure roc didn't think of all these weird form element related special cases. And the draft is wrong about <option>, since that is rendered by css, at least to some extent. I don't think there are other similar special cases.

Jonathon, set assignee to you since you've attached a patch. Please feel free to re-assign. thx!

After experimenting a little further, I'd argue that Blink's behavior regarding innerText for <select> and <option> is really inconsistent/unexpected and should probably -not- be used as a model here. Consider the two testcases: (a) data:text/html,<select size=1><option>foo</option></select> Result: document.getElementsByTagName('option')[0].innerText returns "foo" (b) data:text/html,<select size=2><option>foo</option></select> Result: document.getElementsByTagName('option')[0].innerText returns "" Why the difference? The size attribute causes the <select> to display its options as a list rather than a pop-up, but that's no reason to change the behavior of innerText on the <option> element(s) within it. And then there's the anomaly mentioned earlier: it seems intuitive to expect that the innerText of a parent node should include the innerText of its children. But in Blink this doesn't hold for <select>: (c) data:text/html,<select size=1><option>foo</option></select> Result: document.getElementsByTagName('option')[0].innerText returns "foo" document.getElementsByTagName('option')[0].parentNode.innerText returns "" I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=636322 and https://bugs.chromium.org/p/chromium/issues/detail?id=636327 against Blink for these inconsistencies, which I don't think we should feel compelled to emulate.

Comment on attachment 8779489 [details] [diff] [review] Make innerText return text from <option> elements within a <select>, rather than ignoring them Given the issues above, please reconsider whether we could take this patch, despite the difference from Blink's behavior. IMO, the resulting behavior here is both more intuitive/reasonable and more in line with the draft spec. I suspect there is no real justification for the Blink behavior, it's just an unexamined quirk of the implementation and should be fixed as per the bugs I just filed in the Chromium tracker.

Comment on attachment 8779489 [details] [diff] [review] Make innerText return text from <option> elements within a <select>, rather than ignoring them ok, given that blink behaves so oddly, maybe we can take this.

Cool, thanks. If we do find that it results in new web-compat issues, we can of course reconsider, but I think that's highly unlikely.

So, tryserver informs me that there are testcases in web-platform-tests for innerText on <select>, and the patch here naturally makes them start failing, as they were written to match our old behavior: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c73b05e5f417. What's the appropriate way forward here, then? Given the lack of a clear, official spec for innerText, can we simply adjust the tests to match the new behavior we're implementing? That's easy to do.... but might be unexpected for other vendors when the tests get upstreamed. (But then, should incompletely-spec'd features really have tests in WPT at all?)

Comment on attachment 8779792 [details] [diff] [review] Update web-platform tests for innerText on <select> to match the new behavior So we need to get roc's spec updated too, I think. Not sure how though. Perhaps file a spec bug or comment in that htm spec innerText bug?

I've opened https://github.com/rocallahan/innerText-spec/issues/5 to get Roc's attention (I hope).

https://hg.mozilla.org/integration/mozilla-inbound/rev/da8fd276e566d1b584614041180bbe0ece56bdba Bug 1290937 - Make innerText return text from <option> elements within a <select>, rather than ignoring them. r=smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/08880f8b6043901113740ab0c7d041050f539a3f Bug 1290937 - Update web-platform tests for innerText on <select> to match the new behavior. r=smaug

https://hg.mozilla.org/mozilla-central/rev/da8fd276e566 https://hg.mozilla.org/mozilla-central/rev/08880f8b6043

updating the title of the issue to make it easier to find.

Are there any chances we could uplift this change?