Closed
Bug 327296
Opened 19 years ago
Closed 19 years ago
[BeOS]GetMimeInfoFromExtension is very ineffective
Categories
(Core Graveyard :: File Handling, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: sergei_d, Assigned: sergei_d)
References
Details
(Keywords: fixed1.8.1.8)
Attachments
(1 file)
(deleted),
patch
|
Biesinger
:
review+
dveditz
:
approval1.8.1.8+
|
Details | Diff | Splinter Review |
All code between
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/beos/nsOSHelperAppService.cpp#231
and
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/beos/nsOSHelperAppService.cpp#251
looks weird - it may be replaced with single call BMimeType::GuessMimeType() - native sniffer for file extensions.
It may be main reason for Bug 326898:
Yes, MIME-handling is if I remember correctly done quite inefficient in BeOS, which should be quite good at it.
Also bug 217723, bug 235350 might be interesting to keep in mind.
Assignee | ||
Comment 2•19 years ago
|
||
1)replaces ineffective loops with system calls to handle mimeinfo.
2)changes a bit arguments for be_roster->Launch()
Honestly, all that NS file and mime handling chain is still bit messy for me, so i don't risk atm to change more things, inspite i have that temptation
Assignee: file-handling → sergei_d
Status: NEW → ASSIGNED
Attachment #212389 -
Flags: review?(cbiesinger)
Comment 3•19 years ago
|
||
Comment on attachment 212389 [details] [diff] [review]
patch
+ BMimeType mimeType, tmpType;
+ if( B_OK == mimeType.GuessMimeType(aFileExt, &tmpType))
+ return SetMIMEInfoForType(tmpType.Type(), _retval);
what's the point of tmpType? why not:
BMimeType mimeType;
if (B_OK == BMimeType::GuessMimeType(aFileExt, &mimeType))
(GuessMimeType is a static function)
(also, your style for this if doesn't seem to match the style of the file)
Attachment #212389 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Comment 4•19 years ago
|
||
Per Comment 3:
Probably you are right, will rework/retest it tomorrow
Assignee | ||
Comment 5•19 years ago
|
||
Landed with biesi's corrections:
Checking in mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp,v <-- nsOSHelperAppService.cpp
new revision: 1.16; previous revision: 1.15
done
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 6•19 years ago
|
||
Wondering if there is reason to land it also in 1.8 branch
Comment 7•18 years ago
|
||
(In reply to comment #6)
> Wondering if there is reason to land it also in 1.8 branch
>
wondering the same thing.
Isn't this one a big improvement?
Someone maybe could do a testbuild on 2.0?
Comment 9•18 years ago
|
||
(In reply to comment #8)
> Isn't this one a big improvement?
>
> Someone maybe could do a testbuild on 2.0?
>
Right now, 2.0 will not build under BeOS. We've committed at least 37 bug fixes and enhancements to the trunk but not the branch.
Assignee | ||
Comment 10•18 years ago
|
||
I tried to build SeaMonkey release 1.02 which is also based on some old branch.
It builds, but backporting patches is really troublesome.
Tried to use (with some corrections which allows it building) current widget/src/beos - and got really very crashy version...
I think maybe there were some special changes in branch which our port didn't take in account, but mostly i suspect DnD code around MouseMoved.
So personally I wouldn't spend time on SeaMonkey 1.02 release. Too late.
But FF may be worth some efforts, because there is real difference wuth SeaMonkey-trunk usability, inspite some bugs, and Minefield-BeOS state.
Assignee | ||
Comment 11•17 years ago
|
||
Doug, I think this is one of bugs worth to back-port to branch. In recent SeaMonkey build I "enjoyed" again very slow download manager behavior related to slow icons painting related to this bug, I guess it may be problem in FF too.
Assignee | ||
Comment 12•17 years ago
|
||
Patch applied cleanly to branch as is.
Will test results after remake.
Comment 13•17 years ago
|
||
I haven't experienced the problem you describe with Firefox but I run on fast hardware. I fully support back-porting as many BeOS-only improvements as possible, though. I'll test the patch here as well.
Assignee | ||
Comment 14•17 years ago
|
||
Tested.
Patch is very effective in branch too.
Comment 15•17 years ago
|
||
Tested patch. Confirm operation is correct. Tested against Firefox 2.0.0.8pre branch. Suggest you request inclusion in 1.8.1.8. Thanks for finding this, Sergei!
Updated•17 years ago
|
Attachment #212389 -
Flags: approval1.8.1.8?
Comment 16•17 years ago
|
||
Comment on attachment 212389 [details] [diff] [review]
patch
approved for 1.8.1.8, a=dveditz for release-drivers
Attachment #212389 -
Flags: approval1.8.1.8? → approval1.8.1.8+
Assignee | ||
Comment 17•17 years ago
|
||
Checking in mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/beos/nsOSHelperAppService.cpp,v <-- nsOSHelperAppService.cpp
new revision: 1.13.4.2; previous revision: 1.13.4.1
done
Keywords: fixed1.8.1.8
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•