189295 - nsTextFrame.cpp: The variable isWhitespace has not yet been assigned a value.

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Roland Mainz]

While compiling 2003-01-15-08-trunk on Solaris/SPARC I saw the following
compiler warning in the log:
-- snip --
...
"../../../../../../../../../home/mozilla/src/2003-01-15-08-trunk/mozilla/layout/html/base/src/nsTextFrame.cpp",
line 4845: Warning: 
The variable isWhitespace has not yet been assigned a value.
1 Warning(s) detected.
-- snip --

Looking at the matching code:
-- snip --
4842       }
4843     }
4844 #endif // IBMBIDI
4845     // Get next word/whitespace from the text
4846     PRBool isWhitespace, wasTransformed;
4847     PRInt32 wordLen, contentLen;
4848     union {
4849       char*       bp1;
4850       PRUnichar*  bp2;
4851     };
4852 #ifdef IBMBIDI
4853     wordLen = start;
4854 #endif // IBMBIDI
4855 
4856     // We need to set aTextData.mCanBreakBefore to true after 1st word. But
we can't set 
4857     // aTextData.mCanBreakBefore without seeing the 2nd word. That's
because this frame 
4858     // may only contain part of one word, the other part is in next frame. 
4859     // we don't care if first word is whitespace, that will be addressed
later. 
4860     if (!aTextData.mCanBreakBefore && !firstThing && !isWhitespace) {
4861       firstWordDone = PR_TRUE;
4862     }
4863 
4864     bp2 = aTx.GetNextWord(aTextData.mInWord, &wordLen, &contentLen,
&isWhitespace,
4865                           &wasTransformed, textRun.mNumSegments == 0);
4866 #ifdef IBMBIDI
4867     if (nextBidi) {
4868       mContentLength -= contentLen;
4869 
-- snip --

... this can't be correct - |isWhitespace| is a random value from stack in this
case...;-(

Comment 1

[Roland Mainz]

CC:'ing some people per CVSblame...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, that looks totally wrong.

Shouldn't that check come after the 

bp2 = aTx.GetNextWord(aTextData.mInWord, &wordLen, &contentLen, &isWhitespace,
                      &wasTransformed, textRun.mNumSegments == 0);

call?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Not essential for 1.3beta. I agree with boris, just move the check. Roland, if
you make the patch I'll review it for you.

Comment 4

[Roland Mainz]

Taking... suffering is all mine... ;-/

Comment 5

[Roland Mainz]

Attachment: Patch for 2003-01-20-08-trunk

Comment 6

[Simon Montagu :smontagu]

Comment on attachment 113447 [details] [diff] [review]
Patch for 2003-01-20-08-trunk

I would be much happier if shanjian could sign off on this change.

Comment 7

[Roland Mainz]

Comment on attachment 113447 [details] [diff] [review]
Patch for 2003-01-20-08-trunk

Moving r=-request over to [e-mail] per discussion with smontagu...

Comment 8

[Roland Mainz]

Patch checked-in by timeless
(http://bonsai.mozilla.org/cvsquery.cgi?module=MozillaTinderboxAll&branch=HEAD&cvsroot=/cvsroot&date=explicit&mindate=1046025480&maxdate=1046025720&who=timeless%25mozdev.org),
marking bug as FIXED.

Comment 9

[Roland Mainz]

Comment on attachment 113447 [details] [diff] [review]
Patch for 2003-01-20-08-trunk

Requesting a= for 1.3final branch.

The patch is pretty much low-risk since the previous behaviour was "random"
(based on use of an uninitalised variable), now we (at least) use a initalised
value, e.g. we can't do things much wrong.
And the patch seems to fix the issue that sometimes whitespaces are missing and
textisrenderedwithoutspacesmakingtexthardtoread... :)

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 113447 [details] [diff] [review]
Patch for 2003-01-20-08-trunk

1.3.1 is done. this didn't make it.