826834 - Support sharing HTML so we can share with more apps in Windows 8

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Brian R. Bondy [:bbondy]]

We currently support sharing only what's selected but it is text only.
This should also support HTML text selections.

Comment 1

[Brian R. Bondy [:bbondy]]

I really like this feature by the way for sharing to the mail app and to evernote. I did most of this over the holidays, just finished up today.

Comment 2

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1

Comment 3

[Brian R. Bondy [:bbondy]]

This line is for parity with IE10:
props->put_Description(HStringReference(url.BeginReading()).Get());

It shows up on the sharing panes better for a bunch of apps when you do that.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 698069 [details] [diff] [review]
Patch v1

On the winrt bits, r+. Forwarding the review on the logic in _expandURLs to ehsan.

Comment 5

[Brian R. Bondy [:bbondy]]

ehsan let me know if you can think of other important tags you think we should expand as well assuming the expanding code is OK.

Comment 6

[(no longer active)]

Comment on attachment 698069 [details] [diff] [review]
Patch v1

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

::: widget/windows/winrt/MetroUIUtils.js
@@ +97,5 @@
> +      } else {
> +        this._expandURLs(n.children[i]);
> +      }
> +    }
> +  },

Stealing ideas from <http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsTreeSanitizer.cpp#289>, you want to handle things like:

* form.action
* link.href
* img.longdesc

etc.

Comment 7

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2

expanded support for resolving URLs. 
Also changed to getAttribute/setAttribute because the previous trick no longer worked for some attributes.

Comment 8

[Brian R. Bondy [:bbondy]]

Comment on attachment 698819 [details] [diff] [review]
Patch v2

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

Actually let me replace all those toLower() calls with a single one.

Comment 9

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3.

Comment 10

[(no longer active)]

Comment on attachment 698823 [details] [diff] [review]
Patch v3.

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

Sorry for being pedantic, but this code can be made less horrible if you define a data table like below and just iterate over it.  r=me with that.

{
  'a': 'href',
  'img': ['src', 'longdesc'],
  ...
}

Comment 11

[Brian R. Bondy [:bbondy]]

ok sounds good.

Comment 12

[Brian R. Bondy [:bbondy]]

Implemented comments as per comment 10
https://hg.mozilla.org/projects/elm/rev/6085fdcdfe88

Comment 13

[(no longer active)]

(In reply to comment #12)
> Implemented comments as per comment 10
> https://hg.mozilla.org/projects/elm/rev/6085fdcdfe88

Nit on https://hg.mozilla.org/projects/elm/rev/6085fdcdfe88#l3.19: you should probably use const.

Comment 14

[Brian R. Bondy [:bbondy]]

Good suggestion ,thx:
https://hg.mozilla.org/projects/elm/rev/aa93a7ec2b88

Comment 16

[Brian R. Bondy [:bbondy]]

This was part of the elm -> m-c migration.