Closed Bug 1200602 Opened 4 years ago Closed 4 years ago

Don't show the missing-plugin UI for <applet> with an unknown MIME type

Categories

(Core :: Plug-ins, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: emk, Assigned: emk, Mentored)

References

Details

(Whiteboard: [lang=c++][mentor=kmachulis])

User Story

For an <object>, <embed>, or <applet> with an unrecognized MIME type:

* If fallback content is present, render that (only <object> has fallback content)
* If there is no fallback content, render the element the same way an image with an unrecognized MIME type is rendered. For example see the rendering of data:text/html,<h1>Unknown Image MIME Type<%2Fh1>%0D%0A<p><img src%3D"data%3Aimage%2Funknown%2CHello">

The current behavior of <embed> and <object> with unknown MIME types is correct and should not change. Please test to make sure we don't introduce regressions!

Attachments

(2 files, 1 obsolete file)

Steps to reproduce:
1. Install Win64 Firefox.
2. Open any page that contains a Java applet.

Actual result:
A useless missing plug-in box.

Expected result:
The fallback content should be displayed for web authors to write something more useful.
Microsoft Edge works as expected.
Agreed! I put a spec in the user-story block. I don't think I have paid engineers to work on this right now, but I bet I can find a mentor at least!
User Story: (updated)
Priority: -- → P2
Summary: Win64 Firefox should display fallback content for Java applets and other unsupported plug-ins → Stop showing the missing-plugin UI
This bug will effectively revert bug 425013.
Blocks: 425013
For an <embed>, fallback content will never be present because <embed> has no ability to have fallback content.
We should also change the <applet> behavior.
User Story: (updated)
Hm, looks like an <object> already displays the fallback content correctly. An <embed> cannot have any fallback content. The HTML spec requires to display something visible to user for an unsupported <embed>.
https://html.spec.whatwg.org/multipage/embedded-content.html#the-embed-element:the-embed-element-14
> The embed element has no fallback content. If the user agent can't find a suitable plugin when
> attempting to find and instantiate one for the algorithm above, then the user agent must use a
> default plugin. This default could be as simple as saying "Unsupported Format".
So only an <applet> does matter.
Masatoshi, do you have a link to an test page that has <applet>, <embed>, and/or <object> with unknown MIME types?
User Story: (updated)
Flags: needinfo?(VYV03354)
OS: Windows → All
Hardware: x86_64 → All
Summary: Stop showing the missing-plugin UI → Don't show the missing-plugin UI for <applet> with an unknown MIME type
Whiteboard: [lang=c++][mentor=TBD]
hrm, if there is no fallback content I wonder if we can go to the broken-image icon instead of completely invisible.

I don't care what the HTML spec says, we aren't going to show users any text in this situation.
Whiteboard: [lang=c++][mentor=TBD] → [lang=c++][mentor=kmachulis]
Kyle can you point potential contributors at the place in the code where this would be implemented?
Mentor: kyle
User Story: (updated)
Flags: needinfo?(kyle)
Whiteboard: [lang=c++][mentor=kmachulis] → [lang=c++]
User Story: (updated)
Whiteboard: [lang=c++] → [lang=c++][mentor=kmachulis]
Attached file Testcase
* Edge displays nothing if fallback text is unavailable.
* Internet Explorer 11 works as expected.
* Chrome 44 did not display a missing-plugin box nor fallback text and displayed an infobar.
Flags: needinfo?(VYV03354)
Please test with Java uninstalled because <applet> cannot customize the MIME type.
Ok, so I haven't dealt much this code, but here's my best guess to the future mentee of this bug:

We're interested in the DOM element that loads the plugin or throws an error if it can't be loaded for some reason, which is DOMObjectLoadingContent. The logic for figuring out what we display on various plugin failure cases is in dom/case/DOMObjectLoadingContent.cpp. To display the alternative text we're looking for, we'd usually use the eFallbackAlternate error code. However, the bug here is that we're using some other error code. So, I /think/ fixing this bug means finding the logic related to applets and changing it to use the right error code.
Flags: needinfo?(kyle)
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8657950 - Flags: review?(kyle)
Comment on attachment 8657950 [details] [diff] [review]
Use the alternate content for <applet>

Review of attachment 8657950 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, and seems to work for me! Just have one small comment, and honestly the patch could be checked in this way if the style is ok.

However, before we do a checkin-needed on this, it'd be nice to have a test too, to make sure that we're rendering the right thing. If you look at dom/base/test/test_810494.html, I believe you should be able to adapt that to test for the issues in this bug. Just submit another patch with that test with me as reviewer. If you need more info on mochitests, feel free to fb? me here or I'm qDot on irc, though the MDN mochitest page is pretty through too.

::: dom/base/nsObjectLoadingContent.cpp
@@ +2961,5 @@
>      // determined type should always just be alternate content
>      aType = eFallbackAlternate;
>    }
>  
> +  if (!thisContent->IsHTMLElement(nsGkAtoms::embed) &&

Nit: Ok, so I'd probably err on the side of verbosity here and have

(thisContent->IsHTMLElement(nsGkAtoms::object) ||
thisContent->IsHTMLElement(nsGkAtoms::applet)) &&

as it just makes it more obvious that we want the two tags we know have fallbacks. I realize we probably would never hit this code in the first place in anything that's not an object/applet/embed, though, so the way it currently is would work too. Mostly a style preference issue. :)
Attachment #8657950 - Flags: review?(kyle) → review+
- Test added.
- Changed !isembed to isobject||isapplet.
Attachment #8657950 - Attachment is obsolete: true
Attachment #8659279 - Flags: review?(kyle)
Comment on attachment 8659279 [details] [diff] [review]
Use the alternate content for <applet>, v2

Review of attachment 8659279 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! I'll get a try run going, once that's done we can land it.
Attachment #8659279 - Flags: review?(kyle) → review+
https://hg.mozilla.org/mozilla-central/rev/aa5459a6703d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8659279 [details] [diff] [review]
Use the alternate content for <applet>, v2

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Some web pages will confuse Win64 Fx users if they use a Java applet. A missig plugin box will be displayed, but users have no way to enable Java on Win64 Fx.
[Describe test coverage new/current, TreeHerder]: Tested locally and m-c
[Risks and why]: Low. The new code path is already used for the <object> element.
[String/UUID change made/needed]: none
Attachment #8659279 - Flags: approval-mozilla-aurora?
Comment on attachment 8659279 [details] [diff] [review]
Use the alternate content for <applet>, v2

Improve the quality of the Windows 64 port, taking it.
Attachment #8659279 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1204854
I filed bug 1204854 for comment #6.
You need to log in before you can comment on or make changes to this bug.