Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

getText respects unbreakable whitespace on IE #91

Closed
wants to merge 2 commits into from

2 participants

@cyril-sf

This is a regression from jQuery 1.6.4.

@timmywil
Collaborator

Carriage returns should continue to be removed to make text consistent across browsers. How does removing carriage returns affect non-breaking spaces? Does it match in IE?

@cyril-sf

Actually, I made a mistake, I'm going to fix it. It's not removing carriage returns that fixes the issue, but using nodeValue instead of innerText.

This will look like this

} else if ( typeof elem.innerText === 'string' ) {
// Replace IE's carriage returns
return getText(elem).replace( rReturn, '' );
}

I didn't break any tests, so I didn't realize that my fix removed some useful code.

@timmywil
Collaborator

I see. Are you running the Sizzle tests only? iirc, it may have been a test in jQuery. If it's not actually needed, we can remove it, but I'd like to make sure.

@cyril-sf

Only the Sizzle tests. I'll check with the jQuery tests and update my commit.

@cyril-sf

I ran jQuery tests after having commented the code that removes carriage returns. Nothing broke..

I'm going to keep it anyway to make sure that nothing breaks.

Cyril Fluck getText respects unbreakable whitespace on IE
This is a regression from jQuery 1.6.4.
4260b20
@cyril-sf

I have cleaned the branch and kept the code to remove the carriage returns as discussed even though it may not be tested.

@cyril-sf

I didn't push the right code in the branch initially, there's one more commit to fix a bug in my changes.

@cyril-sf

You can reproduce the bug on IE using this link http://jsfiddle.net/PREjM/5/

@timmywil
Collaborator

innerText is no longer used.

@timmywil timmywil closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 24, 2012
  1. getText respects unbreakable whitespace on IE

    Cyril Fluck authored
    This is a regression from jQuery 1.6.4.
  2. fix bug leading to infinite loop

    Cyril Fluck authored
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 3 deletions.
  1. +2 −2 sizzle.js
  2. +1 −0  test/index.html
  3. +4 −1 test/unit/selector.js
View
4 sizzle.js
@@ -339,12 +339,12 @@ var getText = Sizzle.getText = function( elem ) {
if ( nodeType ) {
if ( nodeType === 1 || nodeType === 9 ) {
- // Use textContent || innerText for elements
+ // Use textContent for elements
if ( typeof elem.textContent === 'string' ) {
return elem.textContent;
} else if ( typeof elem.innerText === 'string' ) {
// Replace IE's carriage returns
- return elem.innerText.replace( rReturn, '' );
+ return getText( elem.childNodes ).replace( rReturn, '' );
} else {
// Traverse it's children
for ( elem = elem.firstChild; elem; elem = elem.nextSibling) {
View
1  test/index.html
@@ -220,6 +220,7 @@ <h2 id="qunit-userAgent"></h2>
</div>
</div>
</dl>
+ <div id="whitespace" style="position:absolute;width:1px;height:1px;overflow:hidden;">Test &nbsp; This</div>
<div id="fx-test-group" style="position:absolute;width:1px;height:1px;overflow:hidden;">
<div id="fx-queue" name="test">
<div id="fadein" class='chain test' name='div'>fadeIn<div>fadeIn</div></div>
View
5 test/unit/selector.js
@@ -406,7 +406,7 @@ test("pseudo - child", function() {
});
test("pseudo - misc", function() {
- expect(19);
+ expect(20);
t( "Headers", ":header", ["qunit-header", "qunit-banner", "qunit-userAgent"] );
t( "Has Children - :has()", "p:has(a)", ["firstp","ap","en","sap"] );
@@ -436,6 +436,9 @@ test("pseudo - misc", function() {
document.body.removeChild( tmp );
+ var whitespace = document.getElementById("whitespace");
+ equal( Sizzle.getText( whitespace ), 'Test ' + String.fromCharCode(160) + ' This' );
+
var input = document.createElement("input");
input.type = "text";
input.id = "focus-input";
Something went wrong with that request. Please try again.