If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

handle known image decoder mime types internally

VERIFIED FIXED in mozilla1.0

Status

()

Core
Plug-ins
P3
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Peter Lubczynski, Assigned: Peter Lubczynski)

Tracking

Trunk
mozilla1.0
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ADT2], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
We need to handle PNG's internally and not let plugins take over.

Need to take a look why nsObjectFrame::IsSupportedImage isn't working for PNGs
like it does GIFs and JPGs.

Comment 1

16 years ago
mng too?
(Assignee)

Updated

16 years ago
(Assignee)

Comment 2

16 years ago
Created attachment 74468 [details] [diff] [review]
possible fix?

Here's a patch that prevents plugins from taking over full-page mode for mime
types that we have image decoders.

This patch comes at a high cost to me as I accidentally nuked my registry and
lost my profiles :(

Grepping my new registry, these look like the type that will be overriden:

image/x-icon
image/bmp
image/gif
image/icon
image/jpeg
video/x-mng
image/x-jng
image/png
image/x-png
image/x-portable-pixmap
image/inspector-bitmap

Does anyone have testcases for any of the above types?
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: nsbeta1, patch
Summary: handle PNG's internally → handle known image decoder mime types internally
Target Milestone: --- → mozilla1.0
(Assignee)

Updated

16 years ago
Blocks: 103934

Comment 3

16 years ago
Comment on attachment 74468 [details] [diff] [review]
possible fix?

No wonder IsSupportedImage does not work -- this is full-page mode :) Although
the patch seems to help, we should probably make this mechanism more flexible
when we come to implementing plugin/mimetype manager.
r=av.
Attachment #74468 - Flags: review+
*** Bug 131844 has been marked as a duplicate of this bug. ***

Comment 5

16 years ago
You would probably do less work with something of this form

  nsCAutoString decoderId = NS_LITERAL_CSTRING("...")
                          + nsDependentString(pluginTag->mMimeTypeArray[i]);

in case the total length goes over the size limit of an auto string.  If it is
impossible to excede this length, then your code is fine.
Please wait with this patch until the patch for bug 104906 is in. It adds
functions to imgILoader for checking if an image type is supported, e.g.:

PRBool isReg;
nsCOMPtr<imgILoader> loader(do_GetService("@mozilla.org/image/loader;1"));
loader->SupportImageWithMimeType(pluginTag->mMimeTypeArray[i], &isReg);
Depends on: 104906

Comment 7

16 years ago
Nominating nsbeta1+ per weekly plugin meeting.
Keywords: nsbeta1 → nsbeta1+

Comment 8

16 years ago
adding in ADT2 as per discussion with beppe.
Whiteboard: [ADT2]
patch for bug 104906 is in, removing dependency
No longer depends on: 104906
(Assignee)

Comment 10

16 years ago
Created attachment 75968 [details] [diff] [review]
patch using |SupportImageWithMimeType| and adding pref

Here's a better patch to use the new |SupportImageWithMimeType|. Just in case
someone ever wants to write a super-duper PNG plugin, this behavior is now
controled by a pref, "plugin.override_internal_types". When set to TRUE, the
old behavior of allowing plugins to override image decoder mime types will be
present. You must restart the browser for the pref to take effect.

Please review/comment.
Attachment #74468 - Attachment is obsolete: true

Comment 11

16 years ago
Maybe it would make sense to check this pref not only on start up but also when
we refresh plugins? Plugin may access this pref during installation process and
ask user if he wants to reconfigure the browser. As an additional benefit there
will be no need for |mOverrideInternalTypes|. I understand that accessing prefs
is kind of expensive but this will not happened often, and besides, I feel that
any prefs related to plugins in general case should be updated with any
potential plugin change, i.e. plugins.refresh.
Comment on attachment 75968 [details] [diff] [review]
patch using |SupportImageWithMimeType| and adding pref

r=biesi
Attachment #75968 - Flags: review+
(Assignee)

Comment 13

16 years ago
There are bugs preventing this pref from being set dynamically! It doesn't
appear that the document loader factories are ever unregistered for either
plugins or image loaders. It doesn't work to set the pref at run-time when the
code is arranged to do it dynamically. We continue to use the plugin or imglib
until a restart.
Keywords: review
Priority: -- → P3

Comment 14

16 years ago
Comment on attachment 75968 [details] [diff] [review]
patch using |SupportImageWithMimeType| and adding pref

OK, that explains special treatment for this case, r=av

Comment 15

16 years ago
Comment on attachment 75968 [details] [diff] [review]
patch using |SupportImageWithMimeType| and adding pref

sr=beard
Attachment #75968 - Flags: superreview+

Comment 16

16 years ago
Comment on attachment 75968 [details] [diff] [review]
patch using |SupportImageWithMimeType| and adding pref

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75968 - Flags: approval+
(Assignee)

Comment 17

16 years ago
patch in trunk, marking FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 18

16 years ago
awesome, fixed on trunk 0326 (win, mac9x, OSx)
Status: RESOLVED → VERIFIED

Comment 19

16 years ago
Confirmed, PNG files do appear to be handled internally by Mozilla rather than
by the QT plug-in (with QT set to not handle them) using FizzillaCFM/2002032915.
You need to log in before you can comment on or make changes to this bug.