296823 - [BEOS] nsWindow::SetTitle should check if mView is NULL

Status: RESOLVED FIXED

(Core Graveyard :: Widget: BeOS, defect)

Description

[tqh]

User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.8b2) Gecko/20050605 Firefox/1.0+
Build Identifier: 

if(text && mView->LockLooper())
should be 
if(text && mView!=NULL && mView->LockLooper())

Reproducible: Always

Comment 1

[tqh]

Attachment: checks if mView is NULL

r?

Comment 2

[Sergei Dolgov]

Comment on attachment 185466 [details] [diff] [review]
checks if mView is NULL

r=sergei_d

though, maybe it is better to keep unified style (which i personally dislike,
btw) when we use:
mView && mView->LockLooper()
everywhere.
With "explicit" comparison better style is classicaly:
NULL != mView (in order to keep it in style with 
NULL == pointer - this last one prevents error when comparison is mistakenly
replaced with assignment)

Comment 3

[Sergei Dolgov]

tqh, what do you think about this version:
NS_METHOD nsWindow::SetTitle(const nsAString& aTitle)
{
	const char *text = ToNewUTF8String(aTitle);
	if(NULL == text)
		return NS_ERROR_FAILURE;
	if(mView && mView->LockLooper())
	{
		mView->Window()->SetTitle(text);
		mView->UnlockLooper();
	}
	delete [] text;
	return NS_OK;
}

Look more in style, don't bother with deleting text if it is NULL.
Only thing which is bit unclear for me - should we return generic
NS_ERROR_FAILURE or NS_ERROR_OUT_OF_MEMORY;
in case of no text

Comment 4

[tqh]

Looks good. Shouldn't NS_ERROR_FAILURE be more appropriate. 

Comment 5

[timeless]

delete[] is not a pair for ToNewUTF8String.

if you just need an auto, try:
NS_ConvertUTF16toUTF8(aTitle).get()

Comment 6

[Doug Shelton]

Attachment: a cleaner implementation

ready to commit?

Comment 7

[Sergei Dolgov]

Comment on attachment 195882 [details] [diff] [review]
a cleaner implementation

r=sergei_d

Comment 8

[Sergei Dolgov]

Comment on attachment 185466 [details] [diff] [review]
checks if mView is NULL

obsoleting

Comment 9

[Sergei Dolgov]

checked in
2005-09-13 13:13 Rev. 1.94 	

Comment 10

[Niels Reedijk]

Shouldn't this bug also be fixed in the MOZILLA_1_8_BRANCH?

Comment 11

[Niels Reedijk]

Requesting permission for the MOZILLA_1_8_BRANCH.

BeOS-only change, will have no effect whatsoever on other plaforms. 

Comment 12

[Asa Dotzler [:asa]]

please request approvals to land in the patch, not with the blocking flag.

Comment 13

[Niels Reedijk]

Sorry, I realised this error when I was lying in bed just now. You fixed it
before me. 

Comment 14

[Niels Reedijk]

Probably not, but since our BeOS Firefox 1.5 tracker depends on this bug, it
keeps  it clear which bugs are fixed for the release and which are not.

Comment 15

[Jesse Ruderman]

Fixed on MOZILLA_1_8_BRANCH.

Checking in nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.91.4.2; previous revision: 1.91.4.1