348447 - feed sanitizer whitelist accessibility and i18n.

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Robert Sayre]

need to go into gkAtoms and the list in nsContentSink.h.

Comment 1

[Robert Sayre]

Mark: bug 340554 landed sanitizing content sinks for the feed API (the comment used to credit you and Sam for the list of tags... missed that while moving things around in a review).

Could you check the whitelist here

http://lxr.mozilla.org/seamonkey/source/content/base/src/nsContentSink.h#130

for accessibility concerns. I'm thinking we should probably add the attributes your role embedding draft.

Comment 2

[Mark Pilgrim (inactive)]

1. Add the "abbr" attribute (not element), which is used by <th> to describe an alternate text representation for a table header, and read by some screenreaders.  See http://diveintoaccessibility.org/19

2. How are namespaced attributes handled?  Does xml:lang get folded into lang?  If not, you'll need to add xml:lang too.

3. Add role attribute, xhtml:role attribute, and xhtml2:role attribute, for DHTML Accessibility, as described here: http://www.w3.org/WAI/PF/GUI/roleTaxonomy-20060508.html  (Note that Firefox recognizes the role attribute in all three namespaces.  The XHTML 2 namespace is defined here: http://lxr.mozilla.org/seamonkey/source/content/base/src/nsNameSpaceManager.cpp#74 )

4. Add all of the attributes in the aaa: namespace (defined here: http://lxr.mozilla.org/seamonkey/source/content/base/src/nsNameSpaceManager.cpp#76 ), as listed in this spec: here:http://www.w3.org/WAI/PF/adaptable/StatesAndProperties-20051106.html#3.2  i.e. allow aaa:checked, aaa:iconed, aaa:disabled, etc.  All of them.

5. If you're going to include the font element and color attribute, you need to include the "bgcolor" and "background" attributes too (otherwise readable text could become unreadable because you stripped one and not the other).

6. If you're stripping the canvas element, be sure you keep its child content, which acts as the fallback content for accessibility purposes.

7. Similarly, you should retain the child content within "noscript", "noembed", and "applet".


Not really accessibility releated (just my unsolicited opinion):

8. If you're going to include the font element and size attribute, you may as well include the "point-size" attribute too.

9. If you're going to include form and input elements, you may as well include the "autocomplete" attribute too.

10. If you're going to include the pre element, you may as well include the "listing" element too.

11. If you're going to include the br element, you may as well include the "nobr" element too.

12. Consider including the marquee element and its attributes "direction", "behavior", "scrolldelay", "scrollamount", and "loop".  Note: that's "direction", not "dir" (which is used elsewhere).

Comment 3

[Robert Sayre]

Attachment: adjust whitelist, deal with QNames in content (thanks W3C!)

Comment 4

[Robert Sayre]

(In reply to comment #2)
> 1. Add the "abbr" attribute...

I covered all of these suggestions, except for allowing marquee and its attributes.

Comment 5

[Robert Sayre]

Comment on attachment 238490 [details] [diff] [review]
adjust whitelist, deal with QNames in content (thanks W3C!)

there are content/ changes in this patch, but they are trivial. SR can check them.

Comment 6

[Benjamin Smedberg]

Comment on attachment 238490 [details] [diff] [review]
adjust whitelist, deal with QNames in content (thanks W3C!)

Please fix the "No newline at end of file" before checkin.

Comment 7

[Robert Sayre]

Comment on attachment 238490 [details] [diff] [review]
adjust whitelist, deal with QNames in content (thanks W3C!)

Hi Jonas, I need SR on the content/ portions of this patch (minor stuff).

Comment 8

[Robert Sayre]

Checking in content/base/src/nsContentSink.h;
/cvsroot/mozilla/content/base/src/nsContentSink.h,v  <--  nsContentSink.h
new revision: 1.18; previous revision: 1.17
done
Checking in content/base/src/nsGkAtomList.h;
/cvsroot/mozilla/content/base/src/nsGkAtomList.h,v  <--  nsGkAtomList.h
new revision: 3.40; previous revision: 3.39
done
Checking in content/xml/document/src/nsXMLFragmentContentSink.cpp;
/cvsroot/mozilla/content/xml/document/src/nsXMLFragmentContentSink.cpp,v  <--  nsXMLFragmentContentSink.cpp
new revision: 1.19; previous revision: 1.18
done
Checking in toolkit/components/feeds/src/FeedProcessor.js;
/cvsroot/mozilla/toolkit/components/feeds/src/FeedProcessor.js,v  <--  FeedProcessor.js
new revision: 1.18; previous revision: 1.17
done
Checking in toolkit/components/feeds/test/test.js;
/cvsroot/mozilla/toolkit/components/feeds/test/test.js,v  <--  test.js
new revision: 1.6; previous revision: 1.5
done
RCS file: /cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/feed_accessible.xml,v
done
Checking in toolkit/components/feeds/test/xml/rfc4287/feed_accessible.xml;
/cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/feed_accessible.xml,v  <--  feed_accessible.xml
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/feed_roleatt.xml,v
done
Checking in toolkit/components/feeds/test/xml/rfc4287/feed_roleatt.xml;
/cvsroot/mozilla/toolkit/components/feeds/test/xml/rfc4287/feed_roleatt.xml,v  <--  feed_roleatt.xml
initial revision: 1.1
done