346586 - Non-PRBool values passed as PRBool in bug 241972

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The patch for bug 241972 passes |aFlags & INTERNAL_LOAD_FLAGS_NEW_WINDOW| as a PRBool.  It's not a boolean value; it shouldn't be being passed to a PRBool arg; doing such things has led to hard-to-track-down bugs in the past.  Please fix accordingly.

Comment 1

[Marria Nazif]

Attachment: make sure to pass a PRBool into DoURILoad

Sorry about that, thanks for catching it

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 231552 [details] [diff] [review]
make sure to pass a PRBool into DoURILoad

Why not just pass in (aFlags & INTERNAL_LOAD_FLAGS_NEW_WINDOW) != 0 ?

If you do want to have the explicit boolean variable, I'm not sure I buy this name.  Something like isFirstLoad would make more sense; see bug 346851.

Comment 3

[Marria Nazif]

Attachment: pass PRBool into DoURILoad

Comment 4

[Marria Nazif]

(In reply to comment #2)
> (From update of attachment 231552 [details] [diff] [review] [edit])
> Why not just pass in (aFlags & INTERNAL_LOAD_FLAGS_NEW_WINDOW) != 0 ?
> 
> If you do want to have the explicit boolean variable, I'm not sure I buy this
> name.  Something like isFirstLoad would make more sense; see bug 346851.
> 

I don't care about having an explicit variable, so I just took it out.
Thanks,
Marria

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 231649 [details] [diff] [review]
pass PRBool into DoURILoad

Looks good.  I assume you need someone to check this in, right?

Comment 6

[Marria Nazif]

(In reply to comment #5)
> (From update of attachment 231649 [details] [diff] [review] [edit])
> Looks good.  I assume you need someone to check this in, right?
> 

I actually have a cvs account, thanks though.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ah, cool.  Thanks for fixing this!

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 231649 [details] [diff] [review]
pass PRBool into DoURILoad

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have done so.

Comment 9

[Marria Nazif]

checked in on trunk and branch