Last Comment Bug 400939 - Browsing through files without extension using FireFTP crashes the browser
: Browsing through files without extension using FireFTP crashes the browser
Status: VERIFIED FIXED
: crash, regression, verified1.8.1.10, verified1.8.1.9
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: unspecified
: x86 OS/2
: -- normal (vote)
: ---
Assigned To: Peter Weilbacher
:
: Nathan Froyd [:froydnj]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-24 03:12 PDT by Peter Weilbacher
Modified: 2007-12-03 15:07 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
likely fix (1.46 KB, patch)
2007-10-24 08:22 PDT, Peter Weilbacher
mozilla: review+
Details | Diff | Splinter Review

Description Peter Weilbacher 2007-10-24 03:12:11 PDT
This was just discussed in the OS/2 newsgroup. Seems like the fix from bug 395576 doesn't properly take into account files without extension.
Comment 1 Peter Weilbacher 2007-10-24 08:22:08 PDT
Created attachment 286010 [details] [diff] [review]
likely fix

We have visited this code three times in the last year, why have we never thought of using stricmp() for the comparison? Even though it's not standardized, it is already used many times in the Mozilla code...

Well, this basically reverts the change from bug 395576 (and hence returns early if there is no extension) and instead uses stricmp which does not touch the input strings. Will ask for review once it's tested.
Comment 2 Peter Weilbacher 2007-10-24 12:36:50 PDT
Comment on attachment 286010 [details] [diff] [review]
likely fix

Mike, I hope you agree that this solution makes a lot more sense than the first try (that didn't check the return codes of either _mbsrchr or strncpy).
Comment 3 Mike Kaply [:mkaply] 2007-10-24 18:38:59 PDT
Comment on attachment 286010 [details] [diff] [review]
likely fix

yeah, that does seem much more logical
Comment 4 Peter Weilbacher 2007-10-25 14:22:57 PDT
Fix checked into trunk and 1.8 branch, and because this was such a stupid and easy to fix regression and it only touches on OS/2 only file I also checked it into the GECKO181_20071004_RELBRANCH.
Comment 5 Daniel Veditz [:dveditz] 2007-10-25 23:43:34 PDT
Please seek approval before checking into a _RELBRANCH even for "not part of build" changes. Seeing unexpected check-ins on "their" branch freaks out the build/release folks.
Comment 6 Peter Weilbacher 2007-10-26 02:29:03 PDT
If it would be clear how the approval process is handled and if it had been announced in .planning that there would be a fast 1.8.1.9 release from the same release branch as 1.8.1.8, I might have done so. So I only saw the unclear message at the top of the 1.8 branch tinderbox, and to me it seemed that if I wanted to have this fix in the FIREFOX_2_0_0_9_RELEASE tag that I had to act now and not wait hours/days/weeks for approval. Would you have seen and reacted to an approval request 5 (?) minutes before tagging?
Comment 7 Peter Weilbacher 2007-12-03 15:07:31 PST
Haven't heard any complaints since this went in.

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