Last Comment Bug 890580 - Enable plaintext serializer to output non-text content as placeholder
: Enable plaintext serializer to output non-text content as placeholder
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Serializers (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla25
Assigned To: Yuan Xulei [:yxl] (Leave from May 18)
:
Mentors:
Depends on:
Blocks: 883129 895239
  Show dependency treegraph
 
Reported: 2013-07-05 20:39 PDT by Yuan Xulei [:yxl] (Leave from May 18)
Modified: 2013-08-09 13:06 PDT (History)
2 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
leo+
wontfix
wontfix
fixed
fixed
wontfix
wontfix
fixed


Attachments
Output non-text content as the placeholder character SUM (2.56 KB, patch)
2013-07-05 22:35 PDT, Yuan Xulei [:yxl] (Leave from May 18)
no flags Details | Diff | Review
test case (4.02 KB, patch)
2013-07-05 22:46 PDT, Yuan Xulei [:yxl] (Leave from May 18)
hsivonen: review-
Details | Diff | Review
Output non-text content as the placeholder character U+FFFC (OBJECT REPLACEMENT CHARACTER). (2.59 KB, patch)
2013-07-08 03:37 PDT, Yuan Xulei [:yxl] (Leave from May 18)
hsivonen: review+
Details | Diff | Review
test case (3.16 KB, patch)
2013-07-08 08:14 PDT, Yuan Xulei [:yxl] (Leave from May 18)
hsivonen: review+
Details | Diff | Review

Description Yuan Xulei [:yxl] (Leave from May 18) 2013-07-05 20:39:37 PDT
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) 2013-07-05 22:35:15 PDT
Created attachment 771735 [details] [diff] [review]
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) 2013-07-05 22:46:32 PDT
Created attachment 771736 [details] [diff] [review]
test case
Comment 3 Henri Sivonen (:hsivonen) 2013-07-08 01:04:34 PDT
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) 2013-07-08 01:08:48 PDT
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) 2013-07-08 03:18:17 PDT
(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) 2013-07-08 03:37:49 PDT
Created attachment 771999 [details] [diff] [review]
Output non-text content as the placeholder character U+FFFC (OBJECT REPLACEMENT CHARACTER).
Comment 7 Yuan Xulei [:yxl] (Leave from May 18) 2013-07-08 08:14:10 PDT
Created attachment 772098 [details] [diff] [review]
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 10 Yuan Xulei [:yxl] (Leave from May 18) 2013-08-07 20:55:06 PDT
This bug should be fixed before the leo+ bug 895239.
Comment 11 Yuan Xulei [:yxl] (Leave from May 18) 2013-08-07 20:56:13 PDT
This bug should be fixed in b2g18 branch as well.
Comment 12 Preeti Raghunath(:Preeti) 2013-08-08 16:26:37 PDT
Moving to leo+ Blocks a blocker: bug 895239

Note You need to log in before you can comment on or make changes to this bug.