254278 - remove some *WithConversion in layout/content

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 155165 [details] [diff] [review]
patch

>Index: content/base/src/nsDocumentEncoder.cpp

>+  AppendUTF16toUTF8(mMimeType, progId);

AppendASCIItoUTF16 would work too, no?	Assuming that's actually a MIME type..

r+sr=bzbarsky either way.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

I'll do the second of the nsPlainTextSerializer functions in Bug 254298, not
here, since it's not currently guaranteed that it's ascii

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

I'm using ToUTF8 to avoid assertions when people put nonascii mimetypes in their
html docs.

checked in.

Comment 5

[Uri Bernstein]

Comment on attachment 155165 [details] [diff] [review]
patch

This patch here regressed bug 299529, and propbably other bidi cases as well:

>   nsIFrame *blockFrame = currentFrame;
>   nsIFrame *thisBlock = nsnull;
>   PRInt32   thisLine;
>-  nsCOMPtr<nsILineIteratorNavigator> it; 
>+  nsILineIteratorNavigator* it;  // This is qi'd off a frame, and those aren't
>+                                 // refcounted
>   result = NS_ERROR_FAILURE;
>   while (NS_FAILED(result) && blockFrame)
>   {
>     thisBlock = blockFrame;
>     blockFrame = blockFrame->GetParent();
>     if (blockFrame) {
>-      it = do_QueryInterface(blockFrame, &result);
>+      CallQueryInterface(blockFrame, &it);
>     }
>   }
>   if (!blockFrame || !it)
>     return NS_ERROR_FAILURE;

Notice that with the patch, |result| is never assigned to inside the loop. As a
result, the loop breaks only when blockFrame is null - which causes the method
to abort with NS_ERROR_FAILURE immediately afterwards. This renders
nsSelection::GetPrevNextBidiLevels() useless.

Comment 6

[Uri Bernstein]

Reopening to get regression fixed (I hope this is proper procedure).

Comment 7

[Boris Zbarsky [:bzbarsky]]

Actually, you should file new bugs on regressions, in general.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: regression patch

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 188858 [details] [diff] [review]
regression patch

fixes a regression with bidi cursor navigation

Comment 10

[Uri Bernstein]

Comment on attachment 188858 [details] [diff] [review]
regression patch

1.8b3 is behind us. Requesting approval1.8b4.

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

regression patch checked in

Checking in layout/generic/nsSelection.cpp;
/cvsroot/mozilla/layout/generic/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.197; previous revision: 3.196
done