Bug Tracker

Opened 9 years ago

Closed 8 years ago

#8111 closed bug (wontfix)

text() reports contents of inline script tags

Reported by: paul.irish Owned by:
Priority: low Milestone: 1.next
Component: manipulation Version: 1.5
Keywords: Cc: dmethvin
Blocked by: Blocking:

Description

test case: http://jsfiddle.net/deadlyicon/TnyPu/

also, using elem.textContent along with elem.innerText should be extremely performant and align more with users expectations.

Change History (11)

comment:1 Changed 9 years ago by Rick Waldron

Component: unfiledmanipulation
Keywords: needsreview added
Priority: undecidedlow
Status: newopen

Confirmed, however this behaviour has existed at least as far back as 1.2.6...

1.2.6 http://jsfiddle.net/rwaldron/TnyPu/2/
1.3.2 http://jsfiddle.net/rwaldron/TnyPu/3/
1.4.3 http://jsfiddle.net/rwaldron/TnyPu/4/
1.4.4 http://jsfiddle.net/rwaldron/TnyPu/5/
1.5.0 http://jsfiddle.net/rwaldron/TnyPu/7/

Changing it now will create a regression.

Last edited 9 years ago by Rick Waldron (previous) (diff)

comment:2 Changed 9 years ago by paul.irish

You say tomatoe, I say tohmahtoe.

You say regression of expected behavior, I say long-standing bug. :)

comment:3 Changed 9 years ago by paul.irish

First draft of perf test: http://jsperf.com/jquery-text

comment:4 Changed 9 years ago by paul.irish

Also it looks like innerText doesn't give text results for hidden elements or script nodes, while textContent does.. which means .textContent aligns with past behavior... fwiw

comment:5 Changed 9 years ago by jitter

  1. As already noted, changing this would be a major bc-breaker. And an unneeded one too
  2. I don't think this is a bug .text() behaves like the specification for textContent (maybe it doesn't in XML context for processing instructions)
  3. Improving the Sizzle.getText function with textContent should be evaluated, but tests are needed to verify the correct text content is returned
  4. http://jsperf.com/jquery-text/2 gives a better look at the possible perf improvements (the first version was also doing the whole selection step at each iteration)
  5. Who uses inline script tags anyway ;)

A little test case to toy with. Shows some bugs in the textFast() as well as the current text() (#6827) method.

comment:6 Changed 9 years ago by paul.irish

comment:7 Changed 8 years ago by dmethvin

Cross-ref #3144 which has another getText implementation that uses textContent and resolves some issues with IE's nom-nominization of whitespace.

comment:8 Changed 8 years ago by addyosmani

Cc: dmethvin added

@dmethvin what were your thoughts on Jitter's comments about this being an unnecessary bc-breaker? Do we still want to pursue a solution for this?

comment:9 Changed 8 years ago by nsfmc

so, i just got bitten by this and assumed it was a straight up error since IE was the only browser doing 'the wrong thing' but after reading the spec for innerText which specifically talks only about returning display != visible... i think that basically this could be resolved via documentation

.text() Get the combined text contents of each visible element in the set of matched elements, including their descendants.

rock on

comment:10 Changed 8 years ago by addyosmani

imo, this should also just be something we document. Leaving open for a second opinion.

comment:11 Changed 8 years ago by dmethvin

Keywords: needsreview removed
Resolution: wontfix
Status: openclosed

With 1.7 we tried using .innerText and we got #11153 for our troubles. So I think we'll stay with the nom-nominization we currently have, which (on the bright side) does use .textContent. At least it's a very consistent bug over time.

Note: See TracTickets for help on using tickets.