Closed Bug 254278 Opened 15 years ago Closed 14 years ago

remove some *WithConversion in layout/content

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.8alpha3

People

(Reporter: Biesinger, Assigned: Biesinger)

References

Details

Attachments

(2 files)

OS: Linux → All
Hardware: PC → All
Summary: remove some → remove some *WithConversion in layout/content
Attachment #155165 - Flags: superreview?(bzbarsky)
Attachment #155165 - Flags: review?(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.
Attachment #155165 - Flags: superreview?(bzbarsky)
Attachment #155165 - Flags: superreview+
Attachment #155165 - Flags: review?(bzbarsky)
Attachment #155165 - Flags: review+
I'll do the second of the nsPlainTextSerializer functions in Bug 254298, not
here, since it's not currently guaranteed that it's ascii
I'm using ToUTF8 to avoid assertions when people put nonascii mimetypes in their
html docs.

checked in.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8alpha3
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.
Reopening to get regression fixed (I hope this is proper procedure).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 299529
Actually, you should file new bugs on regressions, in general.
Attached patch regression patchSplinter Review
Attachment #188858 - Flags: superreview?(bzbarsky)
Attachment #188858 - Flags: review?(bzbarsky)
Attachment #188858 - Flags: superreview?(bzbarsky)
Attachment #188858 - Flags: superreview+
Attachment #188858 - Flags: review?(bzbarsky)
Attachment #188858 - Flags: review+
Comment on attachment 188858 [details] [diff] [review]
regression patch

fixes a regression with bidi cursor navigation
Attachment #188858 - Flags: approval1.8b3?
Comment on attachment 188858 [details] [diff] [review]
regression patch

1.8b3 is behind us. Requesting approval1.8b4.
Attachment #188858 - Flags: approval1.8b3? → approval1.8b4?
Attachment #188858 - Flags: approval1.8b4? → approval1.8b4+
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
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.