The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

Core Graveyard
Widget: BeOS
--
minor
RESOLVED FIXED
12 years ago
2 years ago

People

(Reporter: tqh, Assigned: tqh)

Tracking

({fixed1.8})

Trunk
x86
BeOS
fixed1.8
Dependency tree / graph
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

809 bytes, patch
Sergei Dolgov
: review+
Mike Schroepfer
: approval1.8b5+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
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
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

12 years ago
Created attachment 185466 [details] [diff] [review]
checks if mView is NULL

r?
Assignee: beos → thesuckiestemail
Attachment #185466 - Flags: review?(sergei_d)

Comment 2

12 years ago
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)
Attachment #185466 - Flags: review?(sergei_d) → review+
(Assignee)

Updated

12 years ago
Blocks: 296856
(Assignee)

Updated

12 years ago
Blocks: 266252

Comment 3

12 years ago
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
(Assignee)

Comment 4

12 years ago
Looks good. Shouldn't NS_ERROR_FAILURE be more appropriate. 

Comment 5

12 years ago
delete[] is not a pair for ToNewUTF8String.

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

Comment 6

12 years ago
Created attachment 195882 [details] [diff] [review]
a cleaner implementation

ready to commit?
Attachment #195882 - Flags: review?(sergei_d)

Comment 7

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

r=sergei_d
Attachment #195882 - Flags: review?(sergei_d) → review+

Comment 8

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

obsoleting
Attachment #185466 - Attachment is obsolete: true

Comment 9

12 years ago
checked in
2005-09-13 13:13 Rev. 1.94 	
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 10

12 years ago
Shouldn't this bug also be fixed in the MOZILLA_1_8_BRANCH?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

12 years ago
Requesting permission for the MOZILLA_1_8_BRANCH.

BeOS-only change, will have no effect whatsoever on other plaforms. 
Flags: blocking1.8b5?

Comment 12

12 years ago
please request approvals to land in the patch, not with the blocking flag.
Flags: blocking1.8b5?

Updated

12 years ago
Attachment #195882 - Flags: approval1.8b5?

Comment 13

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

Comment 14

12 years ago
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.

Updated

12 years ago
Flags: blocking1.8b5+

Updated

12 years ago
Attachment #195882 - Flags: approval1.8b5? → approval1.8b5+

Comment 15

12 years ago
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
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.