Last Comment Bug 296823 - [BEOS] nsWindow::SetTitle should check if mView is NULL
: [BEOS] nsWindow::SetTitle should check if mView is NULL
Status: RESOLVED FIXED
: fixed1.8
Product: Core Graveyard
Classification: Graveyard
Component: Widget: BeOS (show other bugs)
: Trunk
: x86 BeOS
: -- minor (vote)
: ---
Assigned To: tqh
: QA timeless
Mentors:
Depends on:
Blocks: 266252 296856
  Show dependency treegraph
 
Reported: 2005-06-06 06:59 PDT by tqh
Modified: 2014-12-09 11:27 PST (History)
4 users (show)
mtschrep: blocking1.8b5+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
checks if mView is NULL (566 bytes, patch)
2005-06-06 07:02 PDT, tqh
sergei_d: review+
Details | Diff | Review
a cleaner implementation (809 bytes, patch)
2005-09-13 10:37 PDT, Doug Shelton
sergei_d: review+
mtschrep: approval1.8b5+
Details | Diff | Review

Description tqh 2005-06-06 06:59:19 PDT
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 2005-06-06 07:02:36 PDT
Created attachment 185466 [details] [diff] [review]
checks if mView is NULL

r?
Comment 2 Sergei Dolgov 2005-06-06 09:49:17 PDT
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 2005-06-22 14:57:22 PDT
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 2005-06-22 23:27:09 PDT
Looks good. Shouldn't NS_ERROR_FAILURE be more appropriate. 
Comment 5 timeless 2005-06-22 23:48:41 PDT
delete[] is not a pair for ToNewUTF8String.

if you just need an auto, try:
NS_ConvertUTF16toUTF8(aTitle).get()
Comment 6 Doug Shelton 2005-09-13 10:37:29 PDT
Created attachment 195882 [details] [diff] [review]
a cleaner implementation

ready to commit?
Comment 7 Sergei Dolgov 2005-09-13 12:41:57 PDT
Comment on attachment 195882 [details] [diff] [review]
a cleaner implementation

r=sergei_d
Comment 8 Sergei Dolgov 2005-09-13 12:44:01 PDT
Comment on attachment 185466 [details] [diff] [review]
checks if mView is NULL

obsoleting
Comment 9 Sergei Dolgov 2005-09-13 13:15:32 PDT
checked in
2005-09-13 13:13 Rev. 1.94 	
Comment 10 Niels Reedijk 2005-09-17 03:01:18 PDT
Shouldn't this bug also be fixed in the MOZILLA_1_8_BRANCH?
Comment 11 Niels Reedijk 2005-09-17 15:28:47 PDT
Requesting permission for the MOZILLA_1_8_BRANCH.

BeOS-only change, will have no effect whatsoever on other plaforms. 
Comment 12 Asa Dotzler [:asa] 2005-09-17 16:34:21 PDT
please request approvals to land in the patch, not with the blocking flag.
Comment 13 Niels Reedijk 2005-09-17 22:55:43 PDT
Sorry, I realised this error when I was lying in bed just now. You fixed it
before me. 
Comment 14 Niels Reedijk 2005-09-18 07:08:31 PDT
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 2005-09-21 22:46:57 PDT
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

Note You need to log in before you can comment on or make changes to this bug.