56019 - JPI on Solaris doesn't recognize plugins with version string in mime-type.

Status: VERIFIED FIXED

(Core Graveyard :: Java: OJI, defect, P3)

Description

[edburns]

Using this markup:

<EMBED type="application/x-java-applet;version=1.3"  CODE = XYZApp.class WIDTH =
100 HEIGHT = 100 model = models/water.xyz  scriptable=false
pluginspage="http://java.sun.com/products/plugin/1.3/plugin-install.html"></EMBED>

Netscape 6 RTM BRANCH from 8 October 2000 and Java Plugin from 2 Oct 2000 the
plugin downloader plugin comes up.

Comment 1

[edburns]

Attachment: test case; tar.gz; unpack; install jpi; visit file:///<UNTARLOCATION>/MoleculeViewer/mime-type.html

Comment 2

[edburns]

Joe, please follow the instructions in the description of attachment
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=16706.

To repro the bug.

Comment 3

[Joe Chou]

The problem here is that plugin uses a ";" to seperate MIME types (see code 1
below) and browser therefore looks for a ";" as the end of a MIME type (see code
2 below). But when a MIME type has a version number, also seperated by a ";",
then the browser failed to parse the MIME types correctly.

The easest (and best?) solution to the problem may be to replace the ";" with a
"|", which is what Windows version uses (that is why Windows version works
currently). But considering that plugin code has been freezed right now, to make
such change for mfs is almost impossible. Therefore, the other solution is to
modify the code in the browser to overcome the ambiguaty of the ";" between two
MIME types and the ";" between a MIME type and its version number. 
The first solution (replacing ";" with "|") is clean and easy, but too late for
now. The second is clugy, but can be done. I propose to use the second solution
as a workaround, and fix the problem with the first solution later.

code 1, plugin seperates MIM types with ";":
(ext/plugin/oji-plugin/src/motif/navig5/JavaPluginFactory5.cpp, l 203)

// (Corresponds to NPP_GetMIMEDescription.)
NS_IMETHODIMP JavaPluginFactory5::GetMIMEDescription(const char **resDesc)
{
    trace("JavaPluginFactory::GetMIMEDescription\n");
    *resDesc = "application/x-java-vm::Java(tm) Plug-in;"PLUGIN_MIMETABLE;

    return NS_OK;
}

code 2, browser, therefore looks for ";" as end of a MIME type:
(module/plugin/nglsrc/nsPluginsDirUNIX.cpp, l 234)
    start = mdesc;
    for(i = 0;i < num && *start;i++) {
        // search start of next token (separator is ';')

        if(i+1 < num) {
            if((nexttoc = PL_strchr(start, ';')) != 0)
                *nexttoc++=0;
            else
                nexttoc=start+strlen(start);
        } else
            nexttoc=start+strlen(start);

        // split string into: mime type ':' extensions ':' description

        mtype = start;
        exten = PL_strchr(start, ':');
        if(exten) {
            *exten++ = 0;
            descr = PL_strchr(exten, ':');
        } else
            descr = NULL;

        if(descr)
            *descr++=0;

#ifdef NS_DEBUG
        printf("Registering plugin for: \"%s\",\"%s\",\"%s\"\n",
               mtype,descr ? descr : "null",exten ? exten : "null");
#endif

        if(!*mtype && !descr && !exten) {
            i--;
            info.fVariantCount--;
        } else {
            info.fMimeTypeArray[i] = mtype ? PL_strdup(mtype) : PL_strdup("");
            info.fMimeDescriptionArray[i] = descr ? PL_strdup(descr) :
                                                    PL_strdup("");
            info.fExtensionArray[i] = exten ? PL_strdup(exten) : PL_strdup("");
        }
        start = nexttoc;
    }

Comment 4

[Joe Chou]

In response to a question about what is the problem:

What I am saying is that currently, the plugin code feeds the MIME type
info in the format of "...type:extension:description;...", using ';' as
 the separator of MIME types. For example,

"...application/x-java-applet::Java(tm)Plug-in;application/x-java-applet;version=1.1::Java(tm)
 Plug-in;...". A ';" can be the separator of two MIME types, or the
 separator of a MIME type's type and its version. That is the ambiguity I
 am talking about. On the other hand, the Window version used a '|' as
 the separator of two MIME types

("...application/x-java-applet::Java(tm)Plug-in|application/x-java-applet;version=1.1::Java(tm)
> Plug-in|..."). Then when the browser parses the MIME type info,  it looks for
'|' as the separator of each MIME type. There is no ambiguity and that is why
the Windows version is working.

Comment 5

[Joe Chou]

Attachment: cvs diff -u output

Comment 6

[Joe Chou]

To eliminate the ambiguity of a ';' being used both for the separator of MIME
types and the separtor of version of a MIME type in the MIME type info received
from plugin, a little routine has been written to replace the MIME type
separator with '|' in the MIME type info. Then the MIME type info is parsed to
retrive each MIME type's type, extension, and description, by running the
existing code.

Comment 7

[edburns]

*** Bug 54424 has been marked as a duplicate of this bug. ***

Comment 8

[edburns]

Joe,

r=edburns

Get r=av and Super review (sr) = brendan and let's get this in the trunk.

Comment 9

[av (gone)]

Now when we get segments separated by '|', should not we also rewrite 
CalculateVariantCount routine? Is my understanding correct that mime type 
version is optional thing and we may have no semicolons (';' that is) at all 
after setMIMETypeSeparator? But CalculateVariantCount goes after this conversion 
in your patch and relies on semicolon as a separator.

Comment 10

[Joe Chou]

CalculateVariantCount() has already been updated (see the first change in the
attachment): instead of looking for ";" as MIME type separator, it looks for "|"
now.

Comment 11

[av (gone)]

Right, I overlooked this. r=av.

Comment 12

[Chris Waterson]

Couple of questions/comments...

- Why are you changing mozilla/configure?

- Have you tested this with legacy plugins to verify it works?

- "void setMIMETypeSeparator()" should be "static void SetMIMETypeSeparator";
  i.e., locally scoped and initial-caps, like CalculateVariantCount (when
  in Rome...)

- Please remove the printf that says "MIME types now separated by blah"

thanks!

Comment 13

[Joe Chou]

Attachment: 2nd round cvs diff -u

Comment 14

[Joe Chou]

- Why are you changing mozilla/configure?
I switched the Perl alias order (from "perl5 perl" to "perl perl5") in configure
in order to load perl properly, since "perl" was used in my build environment.
The change was just for my local use, not part of the fix, and will not be
checked in. 

- Have you tested this with legacy plugins to verify it works?
Yes, and it seemed working OK.

- "void setMIMETypeSeparator()" should be "static void SetMIMETypeSeparator";
  i.e., locally scoped and initial-caps, like CalculateVariantCount (when
  in Rome...)
Done.

- Please remove the printf that says "MIME types now separated by blah"
Done.

The new attachment includes all these changes. Thanks.

Comment 15

[Chris Waterson]

sr=waterson

Comment 16

[edburns]

Fix checked in to trunk.

Comment 17

[Joe Chou]

*** Bug 51630 has been marked as a duplicate of this bug. ***

Comment 18

[Joe Chou]

*** Bug 51630 has been marked as a duplicate of this bug. ***

Comment 19

[Raghu Sakleshpur]

Adding suntracking keyword

Comment 20

[shrirang khanzode]

can someone verify this on solaris ? Thx!

Comment 21

[Joe Chou]

Verify what?

Comment 22

[Margaret Chan]

I've verified the fix and checked in the same fix to the OEM branch.

Comment 23

[Margaret Chan]

Marking verified.