210934 - Option to use Moz to open MNGs even though MNG support dropped

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Ben Hollis]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

There is still an option in preferences/advanced/system to use Moz as the
default handler for MNGs, even though it can't decode them any more.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Bill Mason]

Confirming new, 2003062708 PC/WinXP

Comment 2

[Frank Wein [:mcsmurf]]

This should be a bug? Why, Mozilla doesn't support it offically, so why should
there a ption to open MNG with Mozilla? A XPI could do such a thing, but Mozilla
should not...i'm for resolving invalid in this case, but that's a modul owner thing.

Comment 3

[Bill Mason]

=====
This should be a bug? Why, Mozilla doesn't support it offically, so why should
there a ption to open MNG with Mozilla?
=====
You just answered your own question.  MNG support was removed but the pref to
make Mozilla the default for opening MNG files was not.

Comment 4

[Frank Wein [:mcsmurf]]

Ups, sorry, missunderstood that bug, ok :)

Comment 5

[Frank Wein [:mcsmurf]]

Ok, you have to remove
http://lxr.mozilla.org/mozilla/source/xpfe/components/prefwindow/resources/content/pref-winhooks.xul#50
http://lxr.mozilla.org/mozilla/source/xpfe/components/prefwindow/resources/locale/en-US/pref-winhooks.dtd#16
http://lxr.mozilla.org/mozilla/source/xpfe/components/prefwindow/resources/locale/en-US/pref-winhooks.dtd#17
http://lxr.mozilla.org/mozilla/source/xpfe/components/prefwindow/resources/content/pref-winhooks.js#106
and in
http://lxr.mozilla.org/mozilla/source/xpfe/components/winhooks/nsWindowsHooks.cpp
some lines (also do a search on mng in lxr). I think we don't need any patch for
this now, because until the situation is unclear if NG support comes back to the
trunk. 

Comment 6

[Glenn Randers-Pehrson]

changed OS and Hardware to "all"

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

seems like an xp apps issue to me.

Comment 8

[OstGote!]

See bug 305435 which removed the UI. Note bug 305435 comment 8.

Comment 9

[Peter Weilbacher]

The only file of those listed in comment 5 which still has MNG left is xpfe/components/winhooks/nsWindowsHooks.cpp. Is that going away with suiterunner or will it be copied to suite/? If it isn't going to be copied, then this bug could be marked WFM now that bug 18574 is finally WONTFIXed..

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> The only file of those listed in comment 5 which still has MNG left is
> xpfe/components/winhooks/nsWindowsHooks.cpp. Is that going away with
> suiterunner or will it be copied to suite/? If it isn't going to be copied,
> then this bug could be marked WFM now that bug 18574 is finally WONTFIXed..

AFAIK we will be keeping winhooks when we move to suiterunner. Therefore we will still need to remove the support from that file.

Comment 11

[Peter Weilbacher]

Attachment: First try

It's a bit weird for me to make this patch because I don't have a Windows environment to try to compile it on. But even if I had I could probably not test it as it currently does not seem to get built on trunk at all (winhooks is not listed in any Makefile.in as it still was on the 1.8 branch).

Anyway, this patch takes the naive approach, it should work...

Comment 12

[Mark Banner (:standard8)]

(In reply to comment #11)
> But even if I had I could probably not
> test it as it currently does not seem to get built on trunk at all (winhooks is
> not listed in any Makefile.in as it still was on the 1.8 branch).

I think you'll find it here:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpfe/components/Makefile.in&rev=1.96&mark=109#107

Comment 13

[Peter Weilbacher]

Yeah, thanks for helping a blind man. ;-) I was searching for the filename winhooks instead of the string on LXR...

Comment 14

[Glenn Randers-Pehrson]

The patch looks OK to me.

Comment 15

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 260170 [details] [diff] [review]
First try

You missed some: http://lxr.mozilla.org/seamonkey/search?string=ishandlingmng

Comment 16

[Peter Weilbacher]

Attachment: including isHandlingMNG outside winhooks

Right, this now includes those, too.

Comment 17

[Peter Weilbacher]

While searching for more stuff to remove, I also found
one mention of MNG in xpfe/bootstrap/macbuild/Contents/Info.plist.in. Not sure what that file is used for, though, I have no clue at all about Mac...

Comment 18

[Stefan [:stefanh]]

(In reply to comment #17)
> While searching for more stuff to remove, I also found
> one mention of MNG in xpfe/bootstrap/macbuild/Contents/Info.plist.in. Not sure
> what that file is used for, though, I have no clue at all about Mac...
> 
It can be removed. iirc, suiterunner uses suite/app/macbuild/Contents/Info.plist.in :-)

http://developer.apple.com/documentation/Cocoa/Conceptual/Documents/Concepts/DocTypePList.html

Comment 19

[Stefan [:stefanh]]

I ment that the MNG stuff in the file could be removed, not the file

Comment 20

[Peter Weilbacher]

Attachment: v3, also remove mng from mac

This adds stefanh's recommendation to remove the mng doctype on Mac. Carrying over reviews. I will get this checked in whenever I have a few hours where I can really watch the tinderboxes, but I wouldn't be too unhappy if someone beats me to it...

Comment 21

[Stefan [:stefanh]]

Comment on attachment 262561 [details] [diff] [review]
v3, also remove mng from mac

You forgot http://lxr.mozilla.org/seamonkey/source/suite/app/macbuild/Contents/Info.plist.in#186 ;-)

Comment 22

[Mark Banner (:standard8)]

(In reply to comment #20)
> Created an attachment (id=262561) [details]
> v3, also remove mng from mac
> 
> This adds stefanh's recommendation to remove the mng doctype on Mac. Carrying
> over reviews. I will get this checked in whenever I have a few hours where I
> can really watch the tinderboxes, but I wouldn't be too unhappy if someone
> beats me to it...

I like tidy ups (and had to check something else in) so I've checked this in on Peter's behalf. I also incorporated the change from comment 21, and also generated a new uuid for the nsIWindowsHooksSettings change.

I'll keep an eye on the trees - hopefully they'll stay green ;-) and let Peter mark this fixed when he's happy.

Comment 23

[Peter Weilbacher]

Yes, I don't see a problem with it. Thanks Mark!