Last Comment Bug 296823 - [BEOS] nsWindow::SetTitle should check if mView is NULL
: [BEOS] nsWindow::SetTitle should check if mView is NULL
: fixed1.8
Product: Core Graveyard
Classification: Graveyard
Component: Widget: BeOS (show other bugs)
: Trunk
: x86 BeOS
-- minor (vote)
: ---
Assigned To: tqh
: QA timeless
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: ---

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

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

Comment 2 User image Sergei Dolgov 2005-06-06 09:49:17 PDT
Comment on attachment 185466 [details] [diff] [review]
checks if mView is NULL


though, maybe it is better to keep unified style (which i personally dislike,
btw) when we use:
mView && mView->LockLooper()
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 User image 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)
	if(mView && mView->LockLooper())
	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
in case of no text
Comment 4 User image tqh 2005-06-22 23:27:09 PDT
Looks good. Shouldn't NS_ERROR_FAILURE be more appropriate. 
Comment 5 User image timeless 2005-06-22 23:48:41 PDT
delete[] is not a pair for ToNewUTF8String.

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

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

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

Comment 9 User image Sergei Dolgov 2005-09-13 13:15:32 PDT
checked in
2005-09-13 13:13 Rev. 1.94 	
Comment 10 User image Niels Reedijk 2005-09-17 03:01:18 PDT
Shouldn't this bug also be fixed in the MOZILLA_1_8_BRANCH?
Comment 11 User image 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 User image 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 User image 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 User image 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 User image 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:; previous revision:

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