1173624 - Doorhanger buttons are not rounded on the bottom corners

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Chenxia Liu [:liuche]]

This needs a selector, probably.

Comment 1

[Chenxia Liu [:liuche]]

So this is a bit annoying, because we have 3 button corner states:
1) No rounded corners (non-last doorhanger)
2) One rounded bottom corner (one of two buttons in last doorhanger)
3) Both bottom corners rounded (sole button in last doorhanger)

This means we'll need to have three selectors for each button (positive, negative) and then set that dynamically.

Comment 2

[Sergej Kravcenko]

It will still looks bad with multiple doorhangers when ScrollView is activated. If you scroll buttons, they will overwrite corners anyway. Same with top corners it buttons reach upper edge.

The only solution is somehow clip these corners, but Android doesn't have easy solution.

Comment 3

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f40c2b53715c

Comment 4

[Martyn Haigh (:mhaigh)]

Attachment: Double button

Comment 5

[Martyn Haigh (:mhaigh)]

Attachment: Single button

Comment 6

[Chenxia Liu [:liuche]]

How does this work with long doorhangers and scrolling?

e.g., in PB mode, with TP enabled:
hello.firefox.com, in Site Identity doorhanger

Also, one thing I was concerned about was doorhangers with two sets of buttons, where we don't want to round the middle buttons e.g.,
Save a login on a page (fb.com) with tracking protection enabled - Site Identity doorhanger will show two sets of doorhangers with buttons.

Comment 7

[Martyn Haigh (:mhaigh)]

Attachment: Long doorhanger top

Comment 8

[Martyn Haigh (:mhaigh)]

Attachment: Long doorhanger bottom

Comment 9

[Martyn Haigh (:mhaigh)]

Slight change to the code from that try push - I've moved the rounded corner view from the doorhanger to the anchored_popup

Comment 10

[Chenxia Liu [:liuche]]

Hm, I wonder if we could achieve this with less off-canvas bitmap drawing/perf hit by making 3 kinds of button selectors, with different bottomRight/bottomLeft radius, and then swapping them in. One tricky part is doing this for only the last-added doorhanger (and un-rounding the button corners of the previously-last doorhangers).

Buuuut this is much cleaner in that all the logic is in one confusing place, rather than dynamically changed, which is opening us up for bugs.

Comment 11

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea52dbbb7197

Comment 12

[Martyn Haigh (:mhaigh)]

I'm cautious of over-engineering this - I don't think we have any stats on usage but my guess is that it's not a very commonly used piece of the UI, if I'm right then I think getting it done is better than getting it perfect.  The multiple button selector option sounds complex and time consuming, despite it being a technically more correct way of doing it.

Anyway - patch incoming

Comment 13

[Martyn Haigh (:mhaigh)]

Attachment: MozReview Request: Bug 1173624 - Doorhanger buttons are not rounded on the bottom corners; r?liuche

Bug 1173624 - Doorhanger buttons are not rounded on the bottom corners; r?liuche

Comment 14

[Martyn Haigh (:mhaigh)]

Comment on attachment 8634015 [details]
MozReview Request: Bug 1173624 - Doorhanger buttons are not rounded on the bottom corners; r?liuche

Changing to margaret to review as liuche isn't here atm

Comment 15

[:Margaret Leibovic]

Comment on attachment 8634015 [details]
MozReview Request: Bug 1173624 - Doorhanger buttons are not rounded on the bottom corners; r?liuche

https://reviewboard.mozilla.org/r/13299/#review12003

I think it's good to think about performance/memory concerns, but I agree with mhaigh that this is secondary UI that isn't shown often, so the simplest solution is best. I like that this approach solves the problem without us worrying about different doorhanger/button configurations.

Comment 16

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/6d7cf984b4673fef529827565440cf6b30cd27ad
changeset:  6d7cf984b4673fef529827565440cf6b30cd27ad
user:       Martyn Haigh <[e-mail]>
date:       Tue Jul 14 18:11:49 2015 +0100
description:
Bug 1173624 - Doorhanger buttons are not rounded on the bottom corners r=margaret

Comment 17

[Martyn Haigh (:mhaigh)]

url:        https://hg.mozilla.org/integration/fx-team/rev/8b82cd6f0178faa46f7d6844a8af971663bb26bd
changeset:  8b82cd6f0178faa46f7d6844a8af971663bb26bd
user:       Martyn Haigh <[e-mail]>
date:       Thu Jul 16 11:47:33 2015 +0100
description:
Bug 1173624 - Adding back in incorrect removal of dependancy r=self

Comment 18

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6d7cf984b467
https://hg.mozilla.org/mozilla-central/rev/8b82cd6f0178