Closed Bug 764354 Opened 8 years ago Closed 8 years ago

Flash plugin text "Tap here to activate plugin" is aligned to the left

Categories

(Firefox for Android :: General, defect)

14 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox14 --- verified
firefox15 --- verified
firefox16 --- verified
blocking-fennec1.0 --- betaN+
fennec 14+ ---

People

(Reporter: AdrianT, Assigned: dbaron)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached image screenshot
Firefox Mobile Native 14 beta 7/ Nightly 16.0a1 2012-06-13
Device: Galaxy Nexus (Android 4.0.2)/ HTC Desire (Android 2.2)

Steps to reproduce:
1. Have the Plugins option set to "tap to play".
2. Load youtube.com.
3. Open a flash video.

Expected results:
A placeholder is created instead of the video player asking for user interaction to activate the flash plugin.

Actual results:
The "Tap here to activate plugin" text is not centered. The text is aligned to the left.

Notes:
The issue is not reproducible on Aurora 15.0a2 2012-06-12
Severity: minor → normal
tracking-fennec: --- → ?
blocking-fennec1.0: ? → ---
Is this a problem in beta 6? If this is a new problem that popped up in the latest beta, that's pretty bad.
Confirmed that this is not an issue for me in b6, but is in nightly.
blocking-fennec1.0: --- → ?
I'm making a beta build now to bisect, but we could also figure out when this first appeared in Nightly, then see which of the changesets in the Nightly regression range made their way up to beta.

Kevin pointed me to this range of changesets between the betas:
http://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FENNEC_14_0b6_RELEASE&tochange=FENNEC_14_0b7_RELEASE
For what it's worth, I'm seeing the broken nightly behaviour in yesterday's nightly, built with:

http://hg.mozilla.org/mozilla-central/rev/131961e5e0d1
Turning my font size to "tiny" (i.e. turning off font inflation) fixes this.
kbrosnan - see comment 4 - I'm seeing this on yesterday's nightly, built with 131961e5e0d1, I haven't updated yet.
If this is a font-inflation issue, can't we just add the CSS to keep font-inflation from affecting the layout?
Blocks: 759755
In particular, the part that caused it is this addition in nsHTMLReflowState::InitResizeFlags:

+      nsAutoTArray<nsIFrame*, 32> stack;
+      stack.AppendElement(frame);
+
+      do {
+        nsIFrame *f = stack.ElementAt(stack.Length() - 1);
+        stack.RemoveElementAt(stack.Length() - 1);
+
+        nsIFrame::ChildListIterator lists(f);
+        for (; !lists.IsDone(); lists.Next()) {
+          nsFrameList::Enumerator childFrames(lists.CurrentList());
+          for (; !childFrames.AtEnd(); childFrames.Next()) {
+            nsIFrame* kid = childFrames.get();
+            kid->MarkIntrinsicWidthsDirty();
+            stack.AppendElement(kid);
+          }
+        }
+      } while (stack.Length() != 0);
tracking-fennec: ? → 14+
blocking-fennec1.0: ? → betaN+
At this point in the cycle, we want to fix this in the affected CSS for the plug-in click-to-play prompt. dbaron: can you advise on the best CSS to use? text-size-adjust?
This bug is to make changes to our plugin placeholder to work around the layout issue, creating a clone of this bug to fix the layout issue after this comment.
Assignee: nobody → margaret.leibovic
Attached patch patchSplinter Review
Attachment #632796 - Flags: review?(bzbarsky)
Attachment #632796 - Attachment is patch: true
(In reply to Jet Villegas (:jet) from comment #13)
> At this point in the cycle, we want to fix this in the affected CSS for the
> plug-in click-to-play prompt. dbaron: can you advise on the best CSS to use?
> text-size-adjust?

I don't know of a way to fix this in the CSS.
Comment on attachment 632796 [details] [diff] [review]
patch

[Triage Comment]
Speculatively approved on bz's review - let's get this in everywhere ASAP since it's holding Fennec b7.
Attachment #632796 - Flags: approval-mozilla-beta+
Attachment #632796 - Flags: approval-mozilla-aurora+
Comment on attachment 632796 [details] [diff] [review]
patch

r=me
Attachment #632796 - Flags: review?(bzbarsky) → review+
Assignee: margaret.leibovic → dbaron
I'm actually a little more comfortable with this approach:  being explicit about which reflow states are these "fake" reflow states that we shouldn't do this for.

(At some point we should probably skip all of InitResizeFlags for these, but that's for another day.)

bz... thoughts?  Agree with my preference for this approach?
Attachment #632809 - Flags: review?(bzbarsky)
Attachment #632809 - Flags: review?(bzbarsky) → review+
a=me for whichever patch you two feel is safest, let's get it in post-haste per comment 17.
Oh, and I do think the "maybe safer patch" is probably safer....
Duplicate of this bug: 764510
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/1b87908dc96f/reftest-xul-reflow adds a reftest for this; I'll land it next time I have stuff to land.
Verified on Firefox 14 beta 7 build 2.
Verified fixed on:
- build: Firefox 16.0a1 (2012-07-08) and Firefox 15.0a2 (2012-07-08)
- device: Samsung Galaxy SII
- OS: Android 2.3.4
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.