Closed
Bug 296823
Opened 19 years ago
Closed 19 years ago
[BEOS] nsWindow::SetTitle should check if mView is NULL
Categories
(Core Graveyard :: Widget: BeOS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: thesuckiestemail, Assigned: thesuckiestemail)
References
Details
(Keywords: fixed1.8)
Attachments
(1 file, 1 obsolete file)
809 bytes,
patch
|
sergei_d
:
review+
mtschrep
:
approval1.8b5+
|
Details | Diff | Splinter Review |
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
r?
Assignee: beos → thesuckiestemail
Attachment #185466 -
Flags: review?(sergei_d)
Comment 2•19 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+
Comment 3•19 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
delete[] is not a pair for ToNewUTF8String. if you just need an auto, try: NS_ConvertUTF16toUTF8(aTitle).get()
Comment 7•19 years ago
|
||
Comment on attachment 195882 [details] [diff] [review] a cleaner implementation r=sergei_d
Attachment #195882 -
Flags: review?(sergei_d) → review+
Comment 8•19 years ago
|
||
Comment on attachment 185466 [details] [diff] [review] checks if mView is NULL obsoleting
Attachment #185466 -
Attachment is obsolete: true
Comment 9•19 years ago
|
||
checked in 2005-09-13 13:13 Rev. 1.94
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Comment 10•19 years ago
|
||
Shouldn't this bug also be fixed in the MOZILLA_1_8_BRANCH?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 11•19 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•19 years ago
|
||
please request approvals to land in the patch, not with the blocking flag.
Flags: blocking1.8b5?
Updated•19 years ago
|
Attachment #195882 -
Flags: approval1.8b5?
Comment 13•19 years ago
|
||
Sorry, I realised this error when I was lying in bed just now. You fixed it before me.
Comment 14•19 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•19 years ago
|
Flags: blocking1.8b5+
Updated•19 years ago
|
Attachment #195882 -
Flags: approval1.8b5? → approval1.8b5+
Comment 15•19 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
Closed: 19 years ago → 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Updated•10 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•