964412 - Support basic HTML markup in doorhanger text

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mike Kaply [:mkaply]]

Similar to bug 959880 for the home banner text, we should support basic HTML markup in door hangers.

Comment 1

[Ben Bucksch (:BenB)]

This may be difficult to implement, given that this is a native dialog.
Even if implemented, the set of tags that are allowed should be extremely restricted, basically just the semantic HTML set like <li>, <h1>, <em>, <strong> etc., and maybe <a href="">, but only with simple, checked link targets.

Furthermore, formatted text should be explicitly enabled by the caller (default needs to be plaintext), and callers should take care not to pass untrusted data there.

Comment 2

[:Margaret Leibovic]

This should actually be pretty easy with Html.fromHtml. The API should take care of sanitizing the data for us.

Comment 3

[Andy Pusch [:AndyP]]

I'd be interested in working on this but I'll need more info. Could you tell me what these door hangers are and which files might be affected?

Comment 4

[:Margaret Leibovic]

(In reply to Andy Pusch from comment #3)
> I'd be interested in working on this but I'll need more info. Could you tell
> me what these door hangers are and which files might be affected?

Hi Andy! "Doorhangers" is a term we use to describe the popup notifications that hang off the browser toolbar. Firefox itself will create these notifications for things like a site requesting to use your location, but add-ons can also create them. See:
https://developer.mozilla.org/en-US/Add-ons/Firefox_for_Android/API/NativeWindow/doorhanger

You'll want to look into changing the Doorhanger#setMessage method declared here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/DoorHanger.java#162

To test this, I would make an add-on that uses some HTML mark-up in a doorhanger message. You can just modify this add-on:
https://github.com/mozilla/firefox-for-android-addons/tree/master/boilerplate/native-ui-boilerplate

It already even includes a doorhanger API call:
https://github.com/mozilla/firefox-for-android-addons/blob/master/boilerplate/native-ui-boilerplate/bootstrap.js#L24

Comment 5

[Andy Pusch [:AndyP]]

Attachment: bug964412_BasicHTMLMarkupInDoorHangers.diff

Comment 6

[Andy Pusch [:AndyP]]

Attachment: doorHangerTest.png

HTML markup test screenshot.

Comment 7

[:Margaret Leibovic]

Comment on attachment 8561109 [details] [diff] [review]
bug964412_BasicHTMLMarkupInDoorHangers.diff

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

Nice! Thanks for the screenshot, looks like you did a good job testing :)

It looks like subtext/supertext don't work well, but that's probably an edgecase we don't really need to support.

Comment 8

[Andy Pusch [:AndyP]]

Here is the try link, I hope the settings are alright. :)

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

Comment 9

[:Margaret Leibovic]

(In reply to Andy Pusch from comment #8)
> Here is the try link, I hope the settings are alright. :)
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=c32e27e4f3bf

Awesome, that's perfect!

This is looking good so far, you can add the checkin-needed keyword to the bug once the test runs finish (and look green).

Comment 10

[Andy Pusch [:AndyP]]

There are 2 busted and 1 failed. I don't know if this is relevant to my changes, could you please look at it?

Comment 11

[:Margaret Leibovic]

(In reply to Andy Pusch from comment #10)
> There are 2 busted and 1 failed. I don't know if this is relevant to my
> changes, could you please look at it?

They don't look related, and the busted ones actually passed on a re-try. So you're good to go!

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/23c7c953aab4

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/23c7c953aab4