573316 - isspace assertion failure [@ nsSVGLength::SetValueAsString]

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase (causes Windows assertion dialog in debug builds)

On Windows, isspace asserts if you give it a negative number (other then EOF?).

Comment 1

[Jesse Ruderman]

Attachment: minimal patch

1) NS_IS_SPACE(*number)

2) isspace((unsigned char)(*number))

3) Custom IsSpace like at
http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILParserUtils.cpp#67

I implemented (1) because it was the first thing I thought of.  (3) is probably best because it lets us match the spec.  Do SMIL and SVG have the same concept of what is a space character?

Comment 2

[Robert Longson [:longsonr]]

White space in lists is defined as one or more of the following consecutive characters: "space" (Unicode code 32), "tab" (9), "line feed" (10), "carriage return" (13) and "form-feed" (12).

Comment 3

[Jesse Ruderman]

http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGNumber.cpp#140 also uses isspace.  What definition of "space" does it want? ;)

Comment 4

[Robert Longson [:longsonr]]

nsSVGNumber and nsSVGLength want the same definition, that of comment 2. I think there's a function in nsSVGUtils.h for that - IsSVGWhitespace.

Comment 5

[Jesse Ruderman]

Attachment: patch

Comment 6

[Jesse Ruderman]

Excellent, thanks Robert.  IsSVGWhitespace is apparently unused at the moment, but that's easily remedied ;)

Comment 7

[Daniel Holbert [:dholbert]]

Looks good to me. :)

FWIW, regarding IsSVGWhitespace being unused -- I initially used IsSVGWhitespace() in the parsing code for <animateMotion>, but I later outsourced that parsing to nsCharSeparatedTokenizer in Bug 562310.  And, unfortunately, nsCharSeparatedTokenizer has its own "isWhitespace" method that doesn't catch linefeeds.[1]

We probably need to also make nsCharSeparatedTokenizer (and its sibling  nsCCharSeparatedTokenizer) have an "SVG mode" to make its isWhitespace method check for linefeeds.  That can be a separate bug, though.

[1] http://hg.mozilla.org/mozilla-central/annotate/e0c8f11c3892/xpcom/ds/nsCharSeparatedTokenizer.h#l157

Comment 8

[Daniel Holbert [:dholbert]]

Filed Bug 573489 on the nsCharSeparatedTokenizer issue described in comment 7.

Comment 9

[Jesse Ruderman]

http://hg.mozilla.org/mozilla-central/rev/0c940e50d738