Closed Bug 1173623 Opened 5 years ago Closed 5 years ago

Update "Learn more" link in Tracking protection to be consistent with other doorhanger links

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 42
Tracking Status
firefox41 --- affected
firefox42 --- verified

People

(Reporter: liuche, Assigned: liuche, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [lang=java][good first bug])

Attachments

(3 files)

When Tracking Protection is enabled, we have a "Learn More" link. The style for this should be made consistent with our other doorhanger links (like "Select another login" or "Edit login" in the login doorhanger).

This bug should:
* Add a link item (like doorhanger_link in login_doorhanger.xml) to default_doorhanger.xml
* Update DefaultDoorHanger.addLink to set the text of the doorhanger_link element instead of mashing the link onto the end of the doorhanger text.
Mentor: liuche
For desktop popup notifications, there's an API built in specifically for adding "Learn more" links. Maybe we should think about making these doorhangers consistent by just making it part of the generic doorhanger API.
Blocks: tp-v1
No longer blocks: doorhanger-redesign
Assignee: nobody → liuche
Bug 1173623 - Update "Learn more" link in Tracking protection to be consistent with other doorhanger links. r=ally
Attachment #8628527 - Flags: review?(ally)
Bug 1179509 - Include title + favicon in Site Identity doorhanger for unencrypted sites. r?ally
Attachment #8628528 - Flags: review?(ally)
(In reply to :Margaret Leibovic from comment #1)
> For desktop popup notifications, there's an API built in specifically for
> adding "Learn more" links. Maybe we should think about making these
> doorhangers consistent by just making it part of the generic doorhanger API.

We currently don't have any "Learn more" links that are created from JS. We do have |actionText| in JS though, which is more general, and allows {title, type, data}.

Actually, in the future, we could combine Link with ActionText, because one is a subset of the other. To do this, I'd remove |addLink| from DefaultDoorhanger and lift |addActionText| from LoginDoorhanger up to their shared parent, Doorhanger, with the default method handling creating links. We'd definitely want to do this if we started adding links from JS-created doorhangers, but it's not necessary right now.
Status: NEW → ASSIGNED
Comment on attachment 8628527 [details]
MozReview Request: Bug 1173623 - Update "Learn more" link in Tracking protection to be consistent with other doorhanger links. r=ally

https://reviewboard.mozilla.org/r/12399/#review11245

Ship It!
Attachment #8628527 - Flags: review?(ally) → review+
Comment on attachment 8628528 [details]
MozReview Request: Bug 1179509 - Include title + favicon in Site Identity doorhanger for unencrypted sites. r?ally

https://reviewboard.mozilla.org/r/12401/#review11247

I'm not sure why you even need review for this.
Attachment #8628528 - Flags: review?(ally) → review+
https://hg.mozilla.org/mozilla-central/rev/d51f71a4ef4f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Verified fixed using:
Device: Nexus 4 (Android 5.1)
Build: Firefox for Android 42.0a1 (2015-07-20)
You need to log in before you can comment on or make changes to this bug.