303779 - PresShell::GetPrimaryFrameFor should return nsIFrame*

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Bastiaan Jacques]

Wallpapering patch for bug 302356 exposed that PresShell::GetPrimaryFrameFor()
always returns NS_OK. PresShell::GetPrimaryFrameFor() merely calls
nsFrameManager::GetPrimaryFrameFor(), so it might as well just return that.

Callers need to be updated, though; this should probably wait for 1.9 to go in.

Comment 1

[Bernd]

I see only two similiar cases which might need wallpapered too.

http://lxr.mozilla.org/mozilla/source/content/events/src/nsEventStateManager.cpp#4637
http://lxr.mozilla.org/mozilla/source/toolkit/components/satchel/src/nsFormFillController.cpp#145

all other places that check for the return value check for frame non nullness too.

Comment 2

[Bastiaan Jacques]

Attachment: fix

This patch removes GetPrimaryFrameFor's second argument (out param) and changes
its return value to nsIFrame*. It also updates all the callers.

Comment 3

[Bastiaan Jacques]

Attachment: whole fix

Previous attachment got cut off.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 191991 [details] [diff] [review]
whole fix

>Index: content/events/src/nsEventStateManager.cpp
>@@ -4629,18 +4625,18 @@ nsEventStateManager::GetDocSelectionLoca

>+    if (isCollapsed /* XXX && startFrame do we care? */) {

I wouldn't bother with that XXX comment.

>Index: layout/base/nsCSSFrameConstructor.cpp
> nsCSSFrameConstructor::CaptureStateForFramesOf(nsIContent* aContent,
>+  if (!frame) {
>+    return NS_ERROR_FAILURE;

This used to return NS_OK in this case, and it should continue doing so.

r+sr=bzbarsky with those nits

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments and tip

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, and the nsIPresShell IID needed changing.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Checked in (with the IID change).

Comment 8

[:Gavin Sharp [email: [e-mail]]]

*** Bug 292002 has been marked as a duplicate of this bug. ***