167762 - Make nsIFormControl::GetType() return int instead of nsresult

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[John Keiser (jkeiser)]

nsIFormControl::GetType() requires an out parameter instead of returning an int.
 It *always* returns a value and *never* returns any nsresult other than NS_OK.
 It should be changed to NS_IMETHOD_(PRInt32).  Not only would it make callers
simpler, it would make things less confusing--"do I have to check the return
value or not?"

Comment 1

[John Keiser (jkeiser)]

Attachment: Patch

This also makes nsIFormControlFrame::GetType() do the same thing.

Comment 2

[Peter Lubczynski]

Comment on attachment 109640 [details] [diff] [review]
Patch

Looks good, r=peterl, but did these two sections accidentally slip in?

>Index: layout/html/style/src/nsCSSFrameConstructor.cpp
>===================================================================
>@@ -4934,7 +4932,6 @@
>     if (!aState.mPseudoFrames.IsEmpty()) { // process pending pseudo frames
>       ProcessPseudoFrames(aPresContext, aState.mPseudoFrames, aFrameItems); 
>     }
>-    isReplaced = PR_TRUE;
> 
>     PRBool allowSubframes = PR_TRUE;
>     if (aPresContext) {
>@@ -7327,6 +7324,9 @@
> {
>+  if (aTag == nsHTMLAtoms::noframes) {
>+    printf("noframes\n");
>+  }
>   // The following code allows the user to specify the base tag
>   // of an element using XBL.  XUL and HTML objects (like boxes, menus, etc.)
>   // can then be extended arbitrarily.

Comment 3

[John Keiser (jkeiser)]

Attachment: Patch v1.1

Excellent catch!

Comment 4

[John Keiser (jkeiser)]

Comment on attachment 109686 [details] [diff] [review]
Patch v1.1

Peter has already reviewed this.

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 109686 [details] [diff] [review]
Patch v1.1

>+   * @return the type to be returned [OUT]

You want to remove the [OUT] no?  And maybe clarify that a tad?  "Return the
type to be returned" sounds a bit flaky.  What about:

  @return the type of the form control.

Comment 6

[kinmoz]

Comment on attachment 109686 [details] [diff] [review]
Patch v1.1

sr=[e-mail]

Just address caillon's issue and check if the following change needs the type
to mType treatment:

@@ -2022,8 +1996,7 @@

Comment 7

[John Keiser (jkeiser)]

Fix checked in.