247981 - necko depends on libpr0n

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

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

so... long ago (2002-03-23 05:21 bug 104906), I made nsUnknownDecoder depend on
imagelib.

this means necko depends on libpr0n -> gfx -> layout. so a standalone necko
build requires building layout. that's not optimal.

Comment 1

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

Attachment: patch

like this, maybe...

creates a new nsIContentSniffer interface which lives in necko, and which is
implemented by libpr0n (imgLoader)

Comment 2

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

Comment on attachment 151382 [details] [diff] [review]
patch

what do you think?

Comment 3

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

Comment on attachment 151382 [details] [diff] [review]
patch

I'll make this enumerate a category instead...

Comment 4

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

Attachment: patch v2

with special thanks to nsICategoryManager and our enumerator interfaces.

Comment 5

[Darin Fisher]

Comment on attachment 151764 [details] [diff] [review]
patch v2

>Index: netwerk/build/Makefile.in
>Index: netwerk/protocol/http/src/nsHttpDigestAuth.cpp
>Index: netwerk/protocol/http/src/nsHttpDigestAuth.h

changes in these files seem like they belong in a different bug.


>Index: netwerk/base/public/nsIContentSniffer.idl

>+interface nsIContentSniffer : nsISupports {

opening bracket on a new line for necko?


>Index: netwerk/streamconv/converters/nsUnknownDecoder.cpp

> PRBool nsUnknownDecoder::SniffForImageMimeType(nsIRequest* aRequest)
...
>+  catMan->EnumerateCategory("content-sniffers", getter_AddRefs(sniffers));

i guess there's no point in specifying that we want content sniffers for
image types.  should SniffForImageMimeType be named differently?


>+    nsXPIDLCString contractid;
>+    nsresult rv = sniffer_id->ToString(getter_Copies(contractid));

nit: call GetData instead so you can take advantage of string sharing.


sr=darin

Comment 6

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

Attachment: final patch

- using TryContentSniffers as the new name for SniffImageType
- uses content-sniffing-services as the category name, to make it clear that
they are used as services, not via CreateInstance

Comment 7

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

checked in

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Does this allow extensions to add arbitrary content sniffing capabilities?  That
doesn't seem like the best of ideas...

Comment 9

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

(In reply to comment #8)
> Does this allow extensions to add arbitrary content sniffing capabilities?  That
> doesn't seem like the best of ideas...

No, just in the case where mozilla does not have a content type available (FTP,
HTTP with no content-type header sent, local files with unknown/no extension)

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Requesting aviary1.0, would like to move this over so that I can use the sniffer
to figure out whether favicon data is a real image or not.  Will fix some of the
issues of junk data getting sent and stored incorrectly.  (sniffer fix is a
byproduct of this patch)

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 153690 [details] [diff] [review]
final patch

a=[e-mail]

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 153690 [details] [diff] [review]
final patch

In on aviary, requesting approval-1.7.3 to keep gecko in sync.

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 153690 [details] [diff] [review]
final patch

a=mkaply for 1.7.3

Comment 14

[Mike Kaply [:mkaply]]

Comment on attachment 153690 [details] [diff] [review]
final patch

I changed my mind.

This changes an interface and gives us no bang for the buck.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ok; Will back this out of the 1.7 branch asap.