The default bug view has changed. See this FAQ.

Alternate content is shown for Flash objects when "Tap to Play" is enabled for plugins

VERIFIED FIXED in mozilla15

Status

()

Core
Plug-ins
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: keeler)

Tracking

({mobile, testcase})

Trunk
mozilla15
ARM
Android
mobile, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 609687 [details]
testcase

I guess this is also a Core->Plugins issue (like bug 739048), rather than Fennec issue, so filing it there.

See testcase, make sure you have "Tap to Play" for plugins enabled in your settings in Fennec to reproduce it.

Expected result:
- A "Tap here to activate plugin" placeholder is shown

Actual result:
- A link is shown which comes from the alternate content inside the <object> tag.

Tested on the Samsung Galaxy Nexus, Android 4.0.2.
(Reporter)

Updated

5 years ago
Blocks: 744060
Created attachment 614629 [details] [diff] [review]
proposed fix

In nsObjectLoadingContent::GetPluginSupportState, if aContent is determined to have alternate content, ePluginOtherState is returned. This prevents ePluginClickToPlay from being returned (which results in the "click/tap to play" box not being shown). This patch returns ePluginClickToPlay if that's what the actual plugin support state is, and follows the old behavior otherwise.
Attachment #614629 - Flags: review?(joshmoz)
(Assignee)

Updated

5 years ago
Assignee: nobody → dkeeler
(Assignee)

Updated

5 years ago
Duplicate of this bug: 744197
(Assignee)

Updated

5 years ago
Blocks: 738698

Comment 3

5 years ago
Comment on attachment 614629 [details] [diff] [review]
proposed fix

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

::: content/base/src/nsObjectLoadingContent.cpp
@@ +1991,5 @@
>    }
>  
> +  PluginSupportState pluginDisabledState = GetPluginDisabledState(aContentType);
> +  if (pluginDisabledState == ePluginClickToPlay) return pluginDisabledState;
> +  else return (hasAlternateContent ? ePluginOtherState : pluginDisabledState);

Please use the if/else structure used elsewhere in this code, and always with scope braces.

if () {
  ...
} else {
  ...
}
Attachment #614629 - Flags: review?(joshmoz) → review+
Created attachment 617999 [details] [diff] [review]
fix v2 + test case

Asking for review again to make sure I interpreted your feedback correctly. Also, I included a mochi test case.
Attachment #614629 - Attachment is obsolete: true
Attachment #617999 - Flags: review?(joshmoz)

Updated

5 years ago
Attachment #617999 - Flags: review?(joshmoz) → review+
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=3a1b3ce0b0bc
Created attachment 618844 [details] [diff] [review]
fix + test

Re-uploading patch to properly set title (to prepare for checkin-needed).
Carrying over r+ from joshmoz.
Attachment #617999 - Attachment is obsolete: true
Attachment #618844 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5e876a76c57
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/c5e876a76c57
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 9

5 years ago
Works now on Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20120429 Firefox/15.0a1 ID:20120429040300
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.