Last Comment Bug 787984 - Add tests for imContentSink.jsm
: Add tests for imContentSink.jsm
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Instant Messaging (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 18.0
Assigned To: Florian Quèze [:florian] [:flo]
: instant-messaging
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-03 11:25 PDT by Florian Quèze [:florian] [:flo]
Modified: 2012-09-04 11:52 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
WIP (10.17 KB, patch)
2012-09-03 11:26 PDT, Florian Quèze [:florian] [:flo]
no flags Details | Diff | Splinter Review
Patch v2 (18.01 KB, patch)
2012-09-03 16:01 PDT, Florian Quèze [:florian] [:flo]
clokep: review+
mconley: feedback+
Details | Diff | Splinter Review

Description Florian Quèze [:florian] [:flo] 2012-09-03 11:25:17 PDT
imContentSink.jsm is the file where we filter out unwanted content from received messages before displaying them, so we should really not let regressions slip into it.
Comment 1 Florian Quèze [:florian] [:flo] 2012-09-03 11:26:52 PDT
Created attachment 657887 [details] [diff] [review]
WIP
Comment 2 Florian Quèze [:florian] [:flo] 2012-09-03 16:01:03 PDT
Created attachment 657938 [details] [diff] [review]
Patch v2

Mike, I think you wrote way more unit tests than Patrick and me, so I'm wondering if you would have some interesting feedback to offer on this.

Note for the code review: The change to newRuleset is just some cleanup. The change to cleanupNode fixes https://bugzilla.instantbird.org/show_bug.cgi?id=962.
Comment 3 Patrick Cloke [:clokep] 2012-09-03 16:55:09 PDT
Comment on attachment 657938 [details] [diff] [review]
Patch v2

+  // Allow id only with numbers.
+  addGlobalAllowedAttribute("id", function(aId) /^[0-9]+$/.test(aId));
Not /^\d+$/?

Besides that it looks OK (and all the tests pass ;)). As far as I can tell this should be a pretty thorough test of the content sink.
Comment 4 Mike Conley (:mconley) - (needinfo me!) 2012-09-04 07:48:51 PDT
Comment on attachment 657938 [details] [diff] [review]
Patch v2

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

Looks OK to me.
Comment 5 Florian Quèze [:florian] [:flo] 2012-09-04 08:00:54 PDT
https://hg.mozilla.org/comm-central/rev/e0f0c746579f

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