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

RESOLVED FIXED in Firefox 42

Status

()

Core
Plug-ins
P2
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: emk, Assigned: emk, Mentored)

Tracking

unspecified
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

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 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.

Comment 1

3 years ago
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
(Assignee)

Comment 2

3 years ago
This bug will effectively revert bug 425013.
Blocks: 425013
(Assignee)

Comment 3

3 years ago
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)
(Assignee)

Comment 4

3 years ago
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]

Comment 6

3 years ago
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]

Comment 7

3 years ago
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]
(Assignee)

Comment 8

3 years ago
Created attachment 8655683 [details]
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)
(Assignee)

Comment 9

3 years ago
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)

Comment 11

3 years ago
Created attachment 8657950 [details] [diff] [review]
Use the alternate content for <applet>
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+
(Assignee)

Comment 13

3 years ago
Created attachment 8659279 [details] [diff] [review]
Use the alternate content for <applet>, v2

- 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+
(Assignee)

Comment 15

3 years ago
Try looks green:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=72a4a2dcca98
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa5459a6703d
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Comment 18

3 years ago
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?
status-firefox42: --- → affected
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+
(Assignee)

Updated

3 years ago
Depends on: 1204854
(Assignee)

Comment 21

3 years ago
I filed bug 1204854 for comment #6.
You need to log in before you can comment on or make changes to this bug.