Closed Bug 327296 Opened 19 years ago Closed 19 years ago

[BeOS]GetMimeInfoFromExtension is very ineffective

Categories

(Core Graveyard :: File Handling, defect)

x86
BeOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sergei_d, Assigned: sergei_d)

References

Details

(Keywords: fixed1.8.1.8)

Attachments

(1 file)

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.
Blocks: 326898
Attached patch patch (deleted) — Splinter Review
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 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+
Per Comment 3: Probably you are right, will rework/retest it tomorrow
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
Wondering if there is reason to land it also in 1.8 branch
(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?
(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.
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.
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.
Patch applied cleanly to branch as is. Will test results after remake.
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.
Tested. Patch is very effective in branch too.
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!
Attachment #212389 - Flags: approval1.8.1.8?
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+
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
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: