Closed Bug 1055598 Opened 6 years ago Closed 6 years ago

Regression: door-hangers and other images used in UI are transparent

Categories

(Firefox for Android :: General, defect)

34 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 34
Tracking Status
firefox34 --- fixed
fennec 34+ ---

People

(Reporter: aaronmt, Assigned: ckitching)

References

(Blocks 1 open bug)

Details

(Keywords: regression, reproducible)

Attachments

(3 files)

Attached image screenshot.png
Currently, door-hangers are transparent.

Last good revision: 0aaa2d3d15cc (2014-08-18)
First bad revision: 111a1da2a95d (2014-08-19)

Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0aaa2d3d15cc&tochange=111a1da2a95d

--
LG Nexus 5 (Android 4.4.4)
Nightly (08/14)
See fxteam pushes at bottom, if it's not new-toolbar, it's one of those https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5fb5adcc3835&tochange=6030bd4e48f8
Bug 1048683 looks like the likely reason here. It changed the background image used in the popup. ckitching, could you have a look at this?
Flags: needinfo?(chriskitching)
Blocks: 1048683
No longer blocks: new-toolbar-v1
Attached patch solution.patchSplinter Review
Testing shows this is definitely the fault of my patch.

The problematic nine-patch is arrow_popup_bg. Here it is before and after my patch:

Before:
https://www.dropbox.com/s/snd2f7wu2tpoh5l/arrow_popup_bg-before.9.png?dl=0

After:
https://www.dropbox.com/s/2ckhptm30c1be14/arrow_popup_bg-after.9.png?dl=0


The script has collapsed the scalable Y-region, which consisted of identical rows. This is ostensibly a safe transformation.

For comedic effect, I tried replacing the single row of pixels that now constitute the Y-scalable region with a pair of identical rows. The result was a DoorHanger which had about double the opacity of the one pictured in comment zero:
https://www.dropbox.com/s/daejuktofddsnjl/TwoPixels.png?dl=0

And this is what you get if you revert the changes to that png entirely:
https://www.dropbox.com/s/ubsnxgw538t1qsc/LotsOfPixels.png?dl=0

Compare these two to the image from Comment 0.

DaFuq?
Do nine-patches somehow work in a completely different way to what I think they do that causes this to all make sense?

One of the following seems to be the answer:

1) I'm just wrong about the safety of this transformation on nine-patches (but the documentation (such as it is) seems to support my claim...)

2) There's some weird bug in the way Android handles nine-patches.

3) Something complicated is happening in the code that uses this nine-patch that's producing this funky behaviour.


The other nine-patches in the patch don't appear to be exhibiting similar insanities, so I'm leaning towards option 3.

With that in mind, here's a patch that reverts the changes to the various arrow_popup_bg drawables (and which definitely fixes the reported problem). Bonus points to anyone who figures out the source of this insanity.
Assignee: nobody → chriskitching
Status: NEW → ASSIGNED
Attachment #8475350 - Flags: review?(lucasr.at.mozilla)
Flags: needinfo?(chriskitching)
(In reply to Chris Kitching [:ckitching] from comment #3)

> The other nine-patches in the patch don't appear to be exhibiting similar
> insanities, so I'm leaning towards option 3.
> 
> With that in mind, here's a patch that reverts the changes to the various
> arrow_popup_bg drawables (and which definitely fixes the reported problem).
> Bonus points to anyone who figures out the source of this insanity.

What can we do to do some manual testing around the other 9 patch pngs, so we don't get surprised later.
Noticed that the web autocompletion popup is affected too. ckitching, I'm starting to think we should simply backout the patch until we figure out what's causing this.
Flags: needinfo?(chriskitching)
Comment on attachment 8475350 [details] [diff] [review]
solution.patch

This is not the only image with side-effects (see screenshot I've just posted).
Attachment #8475350 - Flags: review?(lucasr.at.mozilla)
Summary: Regression: door-hangers are transparent → Regression: door-hangers and other images used in UI are transparent
(In reply to Lucas Rocha (:lucasr) from comment #5)
> Created attachment 8475817 [details]
> Autocompletion background
> 
> Noticed that the web autocompletion popup is affected too. ckitching, I'm
> starting to think we should simply backout the patch until we figure out
> what's causing this.

Sounds like a plan. Pushing a backout now, then. Instead of manually verifying all the images, the way forward seems to be to figure out why this is happening. The script performed the intended modification to the images, so the issue lies in that not being as safe as I thought it was.

This is quite interesting, but not that useful to spend a lot of time on. Nine-patches aren't supposed to behave like this... :/
Flags: needinfo?(chriskitching)
Fixed via backout of bug 1048683
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
tracking-fennec: ? → 34+
Target Milestone: --- → Firefox 34
You need to log in before you can comment on or make changes to this bug.