text in Flash content is mojibake

VERIFIED FIXED in Firefox 27

Status

()

Core
Graphics
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: kbrosnan, Assigned: gw280)

Tracking

({dev-doc-needed, regression, site-compat})

Trunk
mozilla29
ARM
Android
dev-doc-needed, regression, site-compat
Points:
---

Firefox Tracking Flags

(firefox24 unaffected, firefox25+ wontfix, firefox26+ wontfix, firefox27+ verified, firefox28 verified, firefox29 verified, relnote-firefox 26+, fennec+)

Details

(URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 811428 [details]
device-2013-09-27-160641.png

Flash content is displaying mojibake instead of text. The stock chart in the screenshot is Flash.

Comment 1

4 years ago
(In reply to Kevin Brosnan [:kbrosnan] from comment #0)
> Created attachment 811428 [details]
> device-2013-09-27-160641.png
> 
> Flash content is displaying mojibake instead of text. The stock chart in the
> screenshot is Flash.

What set of devices/content/versions/etc. are impacted? With the Summit behind us, will you have the time to identify a regression window?
Flags: needinfo?(kbrosnan)
Keywords: qawanted

Comment 2

4 years ago
Nevermind, I think I just remembered you're on PTO. Hopefully Aaron can help out here.
Flags: needinfo?(kbrosnan) → needinfo?(aaron.train)

Updated

4 years ago
Flags: needinfo?(aaron.train)
Keywords: qawanted
Created attachment 814941 [details]
device-2013-10-09-120801.png

Not entirely sure what I'm looking for here. The chart looks fine to me? Where is mojibake in the screenshot?

From my Galaxy SIV zoomed in on the chart on Nightly

WFM?
George, does this look related your skia font wrapping?
Flags: needinfo?(gwright)

Comment 5

4 years ago
Second to last beta coming up, would love to find out if there's anything low risk we can do before Wednesday.
(Reporter)

Updated

4 years ago
tracking-fennec: ? → ---
status-firefox24: unaffected → ---
status-firefox25: affected → ---
status-firefox26: affected → ---
status-firefox27: affected → ---
tracking-firefox25: ? → ---
Duplicate of this bug: 926031
(Reporter)

Comment 7

4 years ago
Unsure how duping a bug cleared all those flags...
tracking-fennec: --- → ?
status-firefox24: --- → unaffected
status-firefox25: --- → affected
status-firefox26: --- → affected
status-firefox27: --- → affected
tracking-firefox25: --- → ?
(In reply to Kevin Brosnan [:kbrosnan] from comment #7)
> Unsure how duping a bug cleared all those flags...

Glitch in the recent tracking flag migration performed over the weekend. I am going through the list restoring the ones that were acciddently cleared. Thanks for putting these back for us.

dkl
(In reply to Aaron Train [:aaronmt] from comment #3)
> Created attachment 814941 [details]
> device-2013-10-09-120801.png
> 
> Not entirely sure what I'm looking for here. The chart looks fine to me?
> Where is mojibake in the screenshot?
> 
> From my Galaxy SIV zoomed in on the chart on Nightly

I'm also not sure what the bug is here. Kevin, can you provide details please?
Flags: needinfo?(gwright) → needinfo?(kbrosnan)
(Reporter)

Comment 10

4 years ago
Created attachment 817214 [details]
Better screen shot

The text inside the Flash chart is not displayed correctly.
Attachment #811428 - Attachment is obsolete: true
Attachment #814941 - Attachment is obsolete: true
Flags: needinfo?(kbrosnan)
(Reporter)

Comment 11

4 years ago
I can't get a regression range due to bug 890272 a Flash crash on activation from July-3 till July-17 where bug 890272 was fixed. This was introduced somewhere in that two week period.
Keywords: regressionwindow-wanted
We'll track, but this is looking more and more like a wontfix for FF25. Hoping George Wright can take a look and compare to a Desktop flash instance of the same, for reproducibility.
Assignee: nobody → gwright
tracking-firefox25: ? → +
tracking-fennec: ? → 25+

Updated

4 years ago
status-firefox25: affected → wontfix
(Reporter)

Comment 13

4 years ago
Worth a relnote?

"Text in Flash elements may display as boxes on Samsung devices."
relnote-firefox: --- → ?

Updated

4 years ago
tracking-firefox26: --- → ?
tracking-firefox26: ? → +
relnote-firefox: ? → 26+
George - is this on your radar for the upcoming week or two?  Firefox 26 is in beta now and while this has already shipped in FF25 it would still be good to get a fix given the popularity of Samsung devices.
Flags: needinfo?(gwright)
Sure, I'll look at it tomorrow.
Flags: needinfo?(gwright)
tracking-fennec: 25+ → +

Comment 16

4 years ago
Update, I carried out a complete wipe of my Galaxy S3 and installed FF25 and flash 11.1.115.81 and the problem had gone. I then installed FF26 and all still fine. I then did an official OS upgrade to Jelly Bean 4.3 and all was still fine. I can not pin point it but it is almost certainly a 3rd party app causing this behaviour, I am working my way 1 app at a time to try and find it.
(Reporter)

Comment 17

4 years ago
changeset:   137328:28957db0a941
user:        George Wright <gw@gwright.org.uk>
date:        Thu Apr 25 20:47:06 2013 -0400
summary:     Bug 848491 - Re-apply bug 687188 - Expand the gradient cache by 2 to store 0/1 colour stop values for clamping.


This appears to be device independent.
Blocks: 848491
(Reporter)

Comment 18

4 years ago
Any update on comment 15?
Flags: needinfo?(gwright)
Not really, I couldn't reproduce it on my device :( Can I get detailed steps to reproduce?
Flags: needinfo?(gwright)
(In reply to Kevin Brosnan [:kbrosnan] from comment #17)
> changeset:   137328:28957db0a941
> user:        George Wright <gw@gwright.org.uk>
> date:        Thu Apr 25 20:47:06 2013 -0400
> summary:     Bug 848491 - Re-apply bug 687188 - Expand the gradient cache by
> 2 to store 0/1 colour stop values for clamping.
> 
> 
> This appears to be device independent.

Also there's no way that patch is causing this regression. It's likely caused by a related patch, though, which was to update Skia.
(Reporter)

Comment 21

4 years ago
I can reproduce this on a Nexus 4 running Android 4.3 and a HTC t-mobile G2 among other phones. Yes I made a slight mistake in the bisect, the issue lies between  http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5f652fba751d&tochange=28957db0a941
OK great, I will get a nexus 4 then :)
We're now past the point of taking speculative fixes for FF26 or waiting for George to get a device and investigate.  Not marking this tracking for FF27, but a low risk uplift might be considered so please nominate when there's a patch.
status-firefox26: affected → wontfix
(Reporter)

Comment 24

4 years ago
This is an common issue in the support forums.
tracking-firefox27: --- → ?

Updated

4 years ago
tracking-firefox27: ? → +
(Reporter)

Comment 25

4 years ago
Any chance of getting this fixed on Firefox 27? This is a very visible regression.
Flags: needinfo?(gwright)
So here's the problem - we are currently updating our in-tree version of Skia as the one we are currently using is seriously old. The fix for this would be different on the old Skia vs. the new Skia, and I'd have to fix it anyway with the new Skia (which is what I was planning on doing). 

As far as I'm aware, there is no plan right now to land the new Skia on 27 (I believe we're targeting 28), and given our limited team resources I'm not too fond of fixing this same issue twice on different Skia revisions, as all the font management code has changed upstream.

What do we want to do here? Would it be completely crazy to try and target the Skia update for 27?
Flags: needinfo?(gwright)
Component: General → Graphics
Product: Firefox for Android → Core
George is going to investigate a "non upstream update of Skia" solution.  Doing a full update in Beta (or even Aurora) is not the safest approach, but neither is letting this bug stay in the released product for six months.  Keep you posted.

Updated

4 years ago
Duplicate of this bug: 958946
NI on George to see if there are any updates here given we are only a few beta's away before Firefox 27 is shipped ?
Flags: needinfo?(gwright)
I have a hacky fix. It's not ideal but it'll serve to bridge the gap until we can find a more optimal/permanent solution.
Flags: needinfo?(gwright)
https://tbpl.mozilla.org/?tree=Try&rev=cb247d615a35
Created attachment 8360842 [details] [diff] [review]
Forward port old SkFontHost_android
Attachment #8360842 - Flags: review?(snorp)
Comment on attachment 8360842 [details] [diff] [review]
Forward port old SkFontHost_android

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

Ok modulo the one issue. Ugly though.

::: gfx/skia/moz.build
@@ +238,4 @@
>      ]
>      # left out of UNIFIED_SOURCES for now to avoid having to patch skia. Should revisit eventually.
>      SOURCES += [
> +        'src/ports/SkFontHost_android_old.cpp',

Is this really on for Android and Gonk both? Should probably only be for Android.
Attachment #8360842 - Flags: review?(snorp) → review+
(In reply to George Wright (:gw280) from comment #30)
> I have a hacky fix. It's not ideal but it'll serve to bridge the gap until
> we can find a more optimal/permanent solution.

The optimal solution exists once we update Skia to upstream, or is it still as ugly there?
status-firefox28: --- → affected
status-firefox29: --- → affected
https://hg.mozilla.org/integration/mozilla-inbound/rev/c62b23f22aea
(In reply to Milan Sreckovic [:milan] from comment #34)
> (In reply to George Wright (:gw280) from comment #30)
> > I have a hacky fix. It's not ideal but it'll serve to bridge the gap until
> > we can find a more optimal/permanent solution.
> 
> The optimal solution exists once we update Skia to upstream, or is it still
> as ugly there?

Already spoke to Milan offline about this, but posting the reply here for all to see; current skia doesn't fix this issue and in fact makes it worse, as the font code has mostly been refactored upstream. Until we assess the extent of the changes and how it affects font selection on Android, we don't know what the optimal solution will be. My guess is that it will be either to fix the Linux fonthost that is maintained upstream to do font selection the way we'd like it to be done, or implement the skia-npapi ANPTypeface methods using gfxFont then returning an SkTypeface wrapping the cairo_font_face_t that is returned.
https://hg.mozilla.org/mozilla-central/rev/c62b23f22aea
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment on attachment 8360842 [details] [diff] [review]
Forward port old SkFontHost_android

[Approval Request Comment]
Regression caused by (bug #): 903993
User impact if declined: Some fonts won't display correctly in Flash content
Testing completed (on m-c, etc.): m-c tests + local flash tests
Risk to taking this patch (and alternatives if risky): Should be low risk; this code has been shipped before
String or IDL/UUID changes made by this patch: None
Attachment #8360842 - Flags: approval-mozilla-beta?
Attachment #8360842 - Flags: approval-mozilla-aurora?
Comment on attachment 8360842 [details] [diff] [review]
Forward port old SkFontHost_android

Approving given this has been a common issue in support forums. Also requesting QA verification here.
Attachment #8360842 - Flags: approval-mozilla-beta?
Attachment #8360842 - Flags: approval-mozilla-beta+
Attachment #8360842 - Flags: approval-mozilla-aurora?
Attachment #8360842 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f8682f72bc36
https://hg.mozilla.org/releases/mozilla-beta/rev/b0f192de4393
status-firefox27: affected → fixed
status-firefox28: affected → fixed
status-firefox29: affected → fixed

Comment 41

4 years ago
Verified fixed using Nexus 4 (Android 4.4.2) on:
 27.0 Beta 9 (2014-01-24)
 Nightly 29.0a1 (2014-01-23)
 Aurora 28.0a2 (2014-01-23)
Status: RESOLVED → VERIFIED
status-firefox27: fixed → verified
status-firefox28: fixed → verified
status-firefox29: fixed → verified

Comment 42

3 years ago
>Verified fixed using Nexus 4 (Android 4.4.2) on:
> 27.0 Beta 9 (2014-01-24)

I wander since this is before the merge date isn't this fixed/or should be, in current (mobile) Firefox and https://www.mozilla.org/en-US/mobile/27.0/releasenotes/

needs to be updated?

I also saw:
Target Milestone: 	mozilla29

I'm not putting pressure on, keep up the good work. Maybe the fix got disabled for a reason?
(Reporter)

Comment 43

3 years ago
Bugzilla can be hard to decode. Target milestone is the release that the check was first checked into, in this case Firefox 29 was nightly. The "Tracking Flags:" section status-firefox# shows when/if a fix was uplifted. In this case it shows that it was uplifted to Firefox 27 and verified to work.

Updated

3 years ago
Keywords: dev-doc-needed, site-compat
(Reporter)

Comment 44

3 years ago
I don't think this needs docs. It was a couple release regression. Users should just update to a current version from the Play Store.
I'll add this to https://developer.mozilla.org/en-US/Firefox/Releases/25/Site_Compatibility for the record, along with other regressions.
You need to log in before you can comment on or make changes to this bug.