890580 - Enable plaintext serializer to output non-text content as placeholder

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Yuan Xulei [:yxl] (Leave from May 18)]

We need a plaintext serializer to output non-textual element as placeholder character.
For example "<span>good<img src="xxx.png">day</span" will be serialized to "good[placeholder]day", but not "goodday" without placeholder.

The main user case is below.

Firefox OS keyboard treats all text field as plaintext and depends on the plaintext serializer to get the text content of a contentEditable element. When user is editing a rich text field, there is no way for the keyboard to aware of an image(with empty alt and title text) or other non-textual element), as these non-textual elements will be outputted as empty string. For example "go<img>od" will be outputted as "good". It makes troubles for the keyboard. See bug 883129 for details.

Comment 1

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Output non-text content as the placeholder character SUM

The patch adds the flag OutputNonTextContentAsPlaceholder to nsIDocumentEncoder, which can be only used for serializing to plaintext. If the flag is set, all non-textual elements will be outputted as a placeholder character SUB('\x1A').

Hi Henry, could you help review the patch or recommend someone else? Thanks in advance.

Comment 2

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: test case

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 771735 [details] [diff] [review]
Output non-text content as the placeholder character SUM

Why U+001A? Why not U+FFFC (OBJECT REPLACEMENT CHARACTER), which seems to exist for this purpose?

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 771736 [details] [diff] [review]
test case

>+		file_placeholder.png \

Does this file actually need to exist in order to test the serializer?

>+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");

You are not supposed to use UniversalXPConnect in new tests. Please either use SpecialPowers somehow (not sure if SpecialPowers can be used here) or write a mochitest-chrome test instead of a mochitest-plain test.

>+    out = toPlaintext("case1");
>+    expected = "This is a button. Hello!";
>+    is(out, expected, "test with <button>");

Why not just
is(toPlaintext("case1"), "This is a button. Hello!", "test with <button>");
?

Comment 5

[Yuan Xulei [:yxl] (Leave from May 18)]

(In reply to Henri Sivonen (:hsivonen) from comment #3)
> Comment on attachment 771735 [details] [diff] [review]
> Output non-text content as the placeholder character SUM
> 
> Why U+001A? Why not U+FFFC (OBJECT REPLACEMENT CHARACTER), which seems to
> exist for this purpose?

It's really a good idea to use U+FFFC (OBJECT REPLACEMENT CHARACTER) instead. I can't find any character better.

Comment 6

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: Output non-text content as the placeholder character U+FFFC (OBJECT REPLACEMENT CHARACTER).

Comment 7

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: test case

(In reply to Henri Sivonen (:hsivonen) from comment #4)
> Comment on attachment 771736 [details] [diff] [review]
> test case
> 
> >+		file_placeholder.png \
> 
> Does this file actually need to exist in order to test the serializer?
> 

No, it doesn't and is removed from this patch.

> >+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
> 
> You are not supposed to use UniversalXPConnect in new tests. Please either
> use SpecialPowers somehow (not sure if SpecialPowers can be used here) or
> write a mochitest-chrome test instead of a mochitest-plain test.

Use SpecialPowers instead.

> 
> >+    out = toPlaintext("case1");
> >+    expected = "This is a button. Hello!";
> >+    is(out, expected, "test with <button>");
> 
> Why not just
> is(toPlaintext("case1"), "This is a button. Hello!", "test with <button>");
> ?

done.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b7b1fd1f4f7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/df4cc5d63c7d

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b7b1fd1f4f7f
https://hg.mozilla.org/mozilla-central/rev/df4cc5d63c7d

Comment 10

[Yuan Xulei [:yxl] (Leave from May 18)]

This bug should be fixed before the leo+ bug 895239.

Comment 11

[Yuan Xulei [:yxl] (Leave from May 18)]

This bug should be fixed in b2g18 branch as well.

Comment 12

[Preeti Raghunath(:Preeti)]

Moving to leo+ Blocks a blocker: bug 895239

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/40ab2e3076f1
https://hg.mozilla.org/releases/mozilla-b2g18/rev/58a4e9af311f

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/40ab2e3076f1
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/58a4e9af311f