Last Comment Bug 739575 - Alternate content is shown for Flash objects when "Tap to Play" is enabled for plugins
: Alternate content is shown for Flash objects when "Tap to Play" is enabled fo...
Status: VERIFIED FIXED
: mobile, testcase
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla15
Assigned To: David Keeler [:keeler] (use needinfo?)
:
Mentors:
http://paradigit.onlinetouch.nl/
: 744197 (view as bug list)
Depends on:
Blocks: click-to-play 744060
  Show dependency treegraph
 
Reported: 2012-03-27 05:42 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2012-04-29 12:09 PDT (History)
9 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (554 bytes, text/html)
2012-03-27 05:42 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
proposed fix (1.24 KB, patch)
2012-04-12 17:23 PDT, David Keeler [:keeler] (use needinfo?)
jaas: review+
Details | Diff | Splinter Review
fix v2 + test case (4.02 KB, patch)
2012-04-24 12:54 PDT, David Keeler [:keeler] (use needinfo?)
jaas: review+
Details | Diff | Splinter Review
fix + test (4.08 KB, patch)
2012-04-26 16:30 PDT, David Keeler [:keeler] (use needinfo?)
dkeeler: review+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2012-03-27 05:42:00 PDT
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.
Comment 1 David Keeler [:keeler] (use needinfo?) 2012-04-12 17:23:33 PDT
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.
Comment 2 David Keeler [:keeler] (use needinfo?) 2012-04-12 17:24:16 PDT
*** Bug 744197 has been marked as a duplicate of this bug. ***
Comment 3 Josh Aas 2012-04-24 10:45:25 PDT
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 {
  ...
}
Comment 4 David Keeler [:keeler] (use needinfo?) 2012-04-24 12:54:02 PDT
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.
Comment 5 David Keeler [:keeler] (use needinfo?) 2012-04-24 13:35:53 PDT
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=3a1b3ce0b0bc
Comment 6 David Keeler [:keeler] (use needinfo?) 2012-04-26 16:30:45 PDT
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.
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-04-26 17:13:54 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5e876a76c57
Comment 9 alex_mayorga 2012-04-29 12:09:28 PDT
Works now on Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20120429 Firefox/15.0a1 ID:20120429040300

Note You need to log in before you can comment on or make changes to this bug.