Closed Bug 1193337 Opened 6 years ago Closed 6 years ago

Checkboxes in doorhangers are not working

Categories

(Firefox for Android Graveyard :: General, defect)

42 Branch
All
Android
defect
Not set
normal

Tracking

(firefox40 unaffected, firefox41 unaffected, firefox42+ verified, firefox43 verified, fennec42+)

VERIFIED FIXED
Firefox 43
Tracking Status
firefox40 --- unaffected
firefox41 --- unaffected
firefox42 + verified
firefox43 --- verified
fennec 42+ ---

People

(Reporter: u421692, Assigned: mhaigh)

References

Details

(Keywords: regression)

Attachments

(3 files)

1. Open http://popuptest.com/popuptest1.html
2. Try to check and uncheck "Don't ask again for this site"

Expected result:
You can easily change the state of the checkbox.

Actual result:
Checkboxes in doorhangers are not working
PS: The only way I was able to make this work was by changing the orientation of the device, and was only able to check or uncheck once.

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=72835344333f&tochange=15155971639c

regressed from bug 1173624 ?
Martyn, can you look into this?
tracking-fennec: --- → ?
Flags: needinfo?(mhaigh)
It's still possible to change the state of doorhanger dropdown menus (such as input selection in the Firefox Hello doorhanger).
OS: Unspecified → Android
Hardware: Unspecified → All
Bug 1193337 - Checkboxes in doorhangers are not working; r?liuche

I couldn't get the old method to work properly, so I have refactored it, this one should be more performant and easier to understand.
Attachment #8647475 - Flags: review?(liuche)
Assignee: nobody → mhaigh
Status: NEW → ASSIGNED
Flags: needinfo?(mhaigh)
tracking-fennec: ? → 42+
Attachment #8647475 - Flags: review?(liuche) → review+
Comment on attachment 8647475 [details]
MozReview Request: Bug 1193337 - Checkboxes in doorhangers are not working; r?liuche

https://reviewboard.mozilla.org/r/15997/#review14577

Nice!

(If you feel like running any profiling, I'd be curious what the differences between these two are. I wonder if the Path approach is actually more performant because we don't have to copy from another canvas. Although save/restore might be expensive. Android Settings > Dev options > Profile GPU rendering > bars - when the bars cross the green line, we've dropped a frame.)

::: mobile/android/base/widget/RoundedCornerLayout.java:53
(Diff revision 1)
> -        if (changed) {
> +        RectF r = new RectF(0, 0, w, h);

Nit: final

::: mobile/android/base/widget/RoundedCornerLayout.java:61
(Diff revision 1)
> -        Bitmap offscreenBitmap = Bitmap.createBitmap(canvas.getWidth(), canvas.getHeight(), Bitmap.Config.ARGB_8888);
> +        canvas.save();

You could add a check here for whether the size has changed, and skip save/restore/clip if it hasn't.

And if it gets called only when there are size changes, add a comment.
Attached image Old method
This is using the original method
Attached image new method
This is the performance using the new method as detailed in this patch.  We're still crossing the line, but we're doing so less severly and for less frames.
url:        https://hg.mozilla.org/integration/fx-team/rev/2b21997e32784d2c3210d03f64b4a1264b084931
changeset:  2b21997e32784d2c3210d03f64b4a1264b084931
user:       Martyn Haigh <mhaigh@mozilla.org>
date:       Tue Aug 18 15:09:25 2015 +0100
description:
Bug 1193337 - Checkboxes in doorhangers are not working; r=liuche
https://hg.mozilla.org/mozilla-central/rev/2b21997e3278
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Verified as fixed on latest Nightly
Depends on: 1201081
Looks like Fx42 is affected, but this was never uplifted to Fx42. There might be regressions from this change now too.
Duplicate of this bug: 1212965
Comment on attachment 8647475 [details]
MozReview Request: Bug 1193337 - Checkboxes in doorhangers are not working; r?liuche

Approval Request Comment
[Feature/regressing bug #]: Originally regressed by bug 1173624 which landed in 42, but this was mistakenly not uplifted to 42.
[User impact if declined]: Checkboxes in doorhangers will not work, so users will lose some level of control over some of their data choices
[Describe test coverage new/current, TreeHerder]: green Beta try run - https://treeherder.mozilla.org/#/jobs?repo=try&revision=54ff8cec05f9
[Risks and why]: low, has been baking for a full cycle. Bug 1201081 would still be present, but it only affects a small number of devices
[String/UUID change made/needed]: none
Attachment #8647475 - Flags: approval-mozilla-beta?
Recent regression; tracking this here rather than in the duplicate bug.
Comment on attachment 8647475 [details]
MozReview Request: Bug 1193337 - Checkboxes in doorhangers are not working; r?liuche

Green try run on m-b, fix for recent regression. Approving this so it can make it into the next mobile beta build.
Attachment #8647475 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed on Firefox 42 Beta 6
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.