Closed Bug 253481 Opened 20 years ago Closed 19 years ago

xul:textbox attribute to specify what happens to pasted line breaks

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: ted)

References

Details

Attachments

(5 files, 20 obsolete files)

778 bytes, application/vnd.mozilla.xul+xml
Details
10.97 KB, patch
neil
: review+
Details | Diff | Splinter Review
2.71 KB, patch
ted
: review+
Details | Diff | Splinter Review
1.89 KB, patch
ted
: review+
Details | Diff | Splinter Review
3.60 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
XUL textboxes should be able to specify what happens when text containing line
breaks is pasted.  It's silly that Thunderbird has to set a global pref to make
pasting multiple addresses into the address widget work, and it sucks that
Firefox has no way to strip line breaks from address bar pastes.

This attribute could override or replace editor.singleLine.pasteNewlines.  I
think it should replace it.
Changing platform to all.

The main reason for the pref is that there are platform differences.  Is it
realistic to assume that most XUL apps will have separate XUL code for each
platform?  It's probably best to keep the current pref for use as a default, but
allow XUL to override it.  For instance, allowing multiline paste in the urlbar
could solve the "broken url" problem for Windows and Mac (bug 23485; it would
probably be better to fix urlbar paste behavior, but no one seems to have
figured out how to do that), without creating unexpected behavior in other text
fields.

Another attribute that could perhaps be considered at the same time: the word
selection attribute, layout.word_select.stop_at_punctuation.  This would neatly
solve problems like people wanting word selection to stop at punctuation in the
editor, yet select entire urls with a doubleclick in the urlbar (e.g. bug
190615).  Though that might be harder since it would require hooks in the frame
code (the editor paste code is fairly straightforward, but can the frame code
query XUL to see whether it has any related attributes?)
OS: Windows XP → All
Hardware: PC → All
I wrote some code, but I haven't tested it yet.  Here's what I did.  Feel free
to sanity check my logic, hopefully I'll post a patch tonight.

in content/html/content/src/nsHTMLAtomList.h:
   add newlines "newlines"

in editor/idl/nsIPlaintextEditor:
   add a property for setting and getting the "handle newlines" value
   add constants for the various values

in editor/libeditor/text/nsPlainTextEditor.{h,cpp}:
   implement property getter/setter
   respect the pref for the default value

in layout/forms/nsFormControlHelper.{h,cpp}:
   add methods to get the value of the "newlines" attribute

in layout/forms/nsTextControlFrame.cpp ::CreateAnonymousContent:
   // Initialize the plaintext editor
   get the property value
   set the value in the plaintext editor

in editor/libeditor/text/nsTextEditRules.cpp:
   use the plaintext editor value to drive the single line newline behavior
Status: NEW → ASSIGNED
Assignee: mozeditor → ted.mielczarek
Status: ASSIGNED → NEW
Attached file Testcase (obsolete) —
Testcase for the patch I'm going to attach next.
Attached patch editor changes (obsolete) — Splinter Review
Changes to editor.  Adds a "newlineHandling" attribute to nsIPlaintextEditor
(along with enumerated constants), sets the default for text input controls
based on the pref "editor.singleLine.pasteNewlines", and when pasting
multi-line data into single-line controls, uses the value of newlineHandling
for the plain text editor to control the behavior.

I have also added a sixth(!) option for the newline handling, which I still
have to implement: strip newlines and all whitespace surrounding them.	I think
this would make the most sense for the URL bar.
Attached patch content and layout changes (obsolete) — Splinter Review
Content and layout changes.  This basically adds an attribute "newlines" to
html:input type="text" that sets the newlineHandling of the associated plain
text editor.  The valid values for newlines are "intact", "firstline",
"spaces", "strip", "commas", and "stripwhitespace".

I'm not terribly keen on adding an attribute to html:input, so if someone can
point me in a better direction using the editor changes in the other patch, I'd
appreciate it.
Blocks: 264568
Attached patch Updated editor changes (obsolete) — Splinter Review
Implemented the last option, it strips CR/LF and surrounding whitespace. 
That's the option I intend for use in the URL bar.
Attachment #196258 - Attachment is obsolete: true
Attachment #196274 - Flags: review?(brade)
(In reply to comment #5)
> I'm not terribly keen on adding an attribute to html:input, so if someone can
> point me in a better direction using the editor changes in the other patch, I'd
> appreciate it.

Nor am I.  Any chance you could somehow expose the editor to the textbox XBL
binding and set things directly on the editor?  Not sure how, though.
Oh yeah, it looks like I can probably just use the interface added in bug 303727
to get an editor from an input, and set it that way.  The editor changes patch
from here is still valid, but the content/layout stuff can be tossed out.  I'll
write an XBL patch soon for this.
Status: NEW → ASSIGNED
Attachment #196255 - Attachment is obsolete: true
Attachment #196259 - Attachment is obsolete: true
Comment on attachment 196274 [details] [diff] [review]
Updated editor changes

I'm not sure what platforms you have already tested this on; please get some
testing on all platforms before this lands.

In nsTextEditRules.cpp, please renumber the six possible options; start with 1
and put the default item as first in the list.

I'm a little concerned about the eNewlinesStripWhitespace block of code (should
this be a switch?):
 * If you remove all of the whitespace before and after it, won't the words
wrap together?	Is the comment wrong or am I missing something or ?
(In reply to comment #9)
> (From update of attachment 196274 [details] [diff] [review] [edit])
> I'm not sure what platforms you have already tested this on; please get some
> testing on all platforms before this lands.

Tested on win32 only so far, I'll test on linux after I address your changes. 
I'll have to get someone else for Mac coverage.

> In nsTextEditRules.cpp, please renumber the six possible options; start with 1
> and put the default item as first in the list.
> 
> I'm a little concerned about the eNewlinesStripWhitespace block of code (should
> this be a switch?):

I'll make it a switch, I was just reusing the existing code structure for now.

>  * If you remove all of the whitespace before and after it, won't the words
> wrap together?	Is the comment wrong or am I missing something or ?

No, that's correct.  My intent is to use this only for the URL bar.  URLs that
are forcibly wrapped tend to have whitespace along with the newlines  (think IRC
in a terminal window).  In this scenario, I think removing whitespace
surrounding the newlines is the right thing to do.  Keep in mind that this
action will only happen if a xul:textbox is explicitly set to
newlines="stripwhitespace".

I'll address your points in an updated patch soon.

Attached patch textbox XBL binding changes (obsolete) — Splinter Review
Uses the exposed editor field to set the newline handling of the html:input
contained in a xul:textbox.  This won't work in untrusted XUL, due to that bug
about XBL bindings executing in the context of the page they're bound to.
Attached file xul testcase (will not work remotely) (obsolete) —
To try this testcase you'll have to download it and put it somewhere with
chrome privileges.  Sorry.
>No, that's correct.  My intent is to use this only for the URL bar.  URLs that
>are forcibly wrapped tend to have whitespace along with the newlines  (think IRC
>in a terminal window).  In this scenario, I think removing whitespace
>surrounding the newlines is the right thing to do.  Keep in mind that this
>action will only happen if a xul:textbox is explicitly set to
>newlines="stripwhitespace".

Perhaps the enum (eNewlinesStripWhitespace) should be renamed to something like: 
 eNewlinesStripForURLBar
(Maybe someone else can suggest a better one?)

If I see "strip whitespace" I think that it removes all whitespace including
spaces in the middle (which this does not do).

One last nit I noticed:
+  while(wsEnd < tString.Length() && NS_IS_SPACE(tString[wsEnd + 1]))
Please add a space after 'while'.
Thanks!
Comment on attachment 196274 [details] [diff] [review]
Updated editor changes

(remove review request since there will be changes)
Attachment #196274 - Flags: review?(brade)
Fixes all of brade's comments.	The last newline handling option has been
renamed to eNewlinesStripSurroundingWhitespace, which is verbose but works.
Attachment #196274 - Attachment is obsolete: true
Attachment #196489 - Flags: review?(brade)
Attached patch Updated binding changes (obsolete) — Splinter Review
Changed to reflect the naming change in the editor patch.  I'm not hot on the
attribute value I've used here, newlines="stripsurroundingwhitespace" is
awfully verbose.  Suggestions are welcome.
Attachment #196328 - Attachment is obsolete: true
Comment on attachment 196489 [details] [diff] [review]
editor patch with review comments

r=brade
Attachment #196489 - Flags: review?(brade) → review+
Comment on attachment 196490 [details] [diff] [review]
Updated binding changes

Is this going to get enough use to make it more worthwhile than gURLBar.editor.QueryInterface(nsIPlaintextEditor).newlineHandling = nsIPlaintextEditor.eNewlinesStripSurroundingWhitespace; ?
Comment on attachment 196489 [details] [diff] [review]
editor patch with review comments

> #include "nsIDOMKeyEvent.h"
> %}
> 
> [ptr] native nsIDOMKeyEvent(nsIDOMKeyEvent);
Not your fault, but this stuff could be replaced by
interface nsIDOMKeyEvent;
thus making HandleKeyPress scriptable.
>   [noscript]void handleKeyPress(in nsIDOMKeyEvent aKeyEvent);

>+, mNewlineHandling(nsIPlaintextEditor::eNewlinesPasteToFirst)
> , mMaxTextLength(-1)
> , mInitTriggerCounter(0)
Initializers should be in class declaration order. If your compiler didn't warn you, get a better compiler :-P

>+  nsresult prefres;
>+  nsCOMPtr<nsIPrefBranch> prefBranch =
>+    do_GetService(NS_PREFSERVICE_CONTRACTID, &prefres);
>+  if (NS_SUCCEEDED(prefres) && prefBranch)
You don't need to check prefres if you're also doing a null check.

>+  if (! aNewlineHandling)
>+    return NS_ERROR_NULL_POINTER;
NS_ENSURE_ARG_POINTER(aNewlineHandling);

>+    mEditor->GetNewlineHandling(&singleLineNewlineBehavior);
nsTextEditRules is a friend of nsPlaintextEditor so just use switch(mEditor->mNewlineHandling) here and below.
(In reply to comment #18)
> (From update of attachment 196490 [details] [diff] [review] [edit])
> Is this going to get enough use to make it more worthwhile than
> gURLBar.editor.QueryInterface(nsIPlaintextEditor).newlineHandling =
> nsIPlaintextEditor.eNewlinesStripSurroundingWhitespace; ?

Given the number of different interpretations of how newlines should be handled, I think there's enough demand to justify an attribute on xul:textbox.  It's going to be a lot nicer to see <textbox newlines="stripsurroundingwhitespace" id=...> than to have to use JS to define it anywhere we want it.  This will likely affect more than just the URL bar.  I believe Thunderbird sets the newlines pref to handle pasting into the To: textbox, and there's a dependent bug on this for Chatzilla handling newlines in the chat input box.
Attached patch Updated editor patch (obsolete) — Splinter Review
Addresses neil's comments, fixes the array out of bounds in the "strip surrounding whitespace" loop that jag pointed out, added a default case to the switch.
Attachment #196489 - Attachment is obsolete: true
Attachment #201743 - Flags: superreview?
Attachment #201743 - Flags: review+
Attachment #201743 - Flags: superreview? → superreview?(jag)
That'll teach me not to build before I submit a patch.
Attachment #201743 - Attachment is obsolete: true
Attachment #201747 - Flags: superreview?(jag)
Attachment #201747 - Flags: review+
Attachment #201743 - Flags: superreview?(jag)
Attached patch updated binding patch (obsolete) — Splinter Review
Apparently on trunk you need to QI the input field to get at its editor.
Attachment #196490 - Attachment is obsolete: true
Attachment #201754 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch equivalent XPFE binding patch (obsolete) — Splinter Review
Same thing for XPFE.
Attachment #201758 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 201747 [details] [diff] [review]
editor patch without that misplaced comma

Just picking some nits, your patch looks fine otherwise:

>Index: editor/idl/nsIPlaintextEditor.idl
>===================================================================
>RCS file: /cvsroot/mozilla/editor/idl/nsIPlaintextEditor.idl,v
>retrieving revision 1.13
>diff -u -p -5 -r1.13 nsIPlaintextEditor.idl
>--- editor/idl/nsIPlaintextEditor.idl	17 Oct 2005 00:54:02 -0000	1.13
>+++ editor/idl/nsIPlaintextEditor.idl	3 Nov 2005 15:15:49 -0000
>@@ -41,11 +41,11 @@
> #include "nsIDOMKeyEvent.h"
> %}
> 
> [ptr] native nsIDOMKeyEvent(nsIDOMKeyEvent);

Though not related to this patch, could you help us clean this up by changing this to interface nsIDOMKeyEvent? Then you can also remove the above c++ section (I'm assuming that everyone who needs nsIDOMKeyEvent.h is already including it).

>Index: editor/libeditor/text/nsPlaintextEditor.cpp
>===================================================================

>+  // check the "single line editor newline handling" pref
>+  nsresult prefres;
>+  nsCOMPtr<nsIPrefBranch> prefBranch =
>+    do_GetService(NS_PREFSERVICE_CONTRACTID, &prefres);
>+  if (NS_SUCCEEDED(prefres))
>+    prefBranch->GetIntPref("editor.singleLine.pasteNewlines",
>+                           &mNewlineHandling);

Since you don't return |prefres| I'd rather you do

  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));
  if (prefBranch)

>Index: editor/libeditor/text/nsTextEditRules.cpp
>===================================================================

Instead of looking ahead (wsEnd + 1) you can look at the current character to decide when to stop. You'll end up one character past the last character of the range you're interested in. The nice property you then get is that (end - begin) is the length of the range. Here is the changed code:

          PRUint32 wsBegin = firstCRLF, wsEnd = firstCRLF + 1;

          while (wsEnd < tString.Length() && NS_IS_SPACE(tString[wsEnd]))
            ++wsEnd;

          // now cut this range out of the string
          tString.Cut(wsBegin, wsEnd - wsBegin);

sr=jag with these nits addressed.
Attachment #201747 - Flags: superreview?(jag) → superreview+
Attached patch final editor patch (obsolete) — Splinter Review
Addresses jag's comments.  This patch can be checked in without the binding patches, since it checks the pref to use as the default value everything should continue to work as normal.
Attachment #201747 - Attachment is obsolete: true
Attachment #201792 - Flags: superreview+
Attachment #201792 - Flags: review+
For future proofing, you could require that all newlines attribute values be names taken from the .idl but without the "eNewline" prefix, like so:
const nsIPlaintextEditor = Components.interfaces.nsIPlaintextEditor;
const nsIDOMNSEditableElement = Components.interfaces.nsIDOMNSEditableElement;
str = "eNewline" + this.getAttribute('newlines');
if (str in nsIPlaintextEditor)
  this.inputField.QueryInterface(nsIDOMNSEditableElement)
      .editor.QueryInterface(nsIPlaintextEditor)
      .newlineHandling = nsIPlaintextEditor[str];
Neil: my only issue with that is that either the attribute values then have to be MixedCase, or we have to rename the enumerated values to be lowercase, both of which contradict traditional style.  I do agree that it's a nicer solution, however.  Any ideas?
Add a lookup table that maps the lowercase strings to the enums.
Attached patch more final editor patch (obsolete) — Splinter Review
With jag's other nit addressed.  jag, want to make sure you're happy with it?
Attachment #201792 - Attachment is obsolete: true
Attachment #202039 - Flags: superreview?(jag)
Attachment #202039 - Flags: review+
Attached patch reworked toolkit binding patch (obsolete) — Splinter Review
Reworked to be more spiffy
Attachment #201754 - Attachment is obsolete: true
Attachment #202040 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #201754 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #201758 - Attachment is obsolete: true
Attachment #201758 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #196330 - Attachment is obsolete: true
Comment on attachment 202040 [details] [diff] [review]
reworked toolkit binding patch

>+              if(x.match(/^eNewlines/)) {
match is inappropriate for a boolean test.

>+                if(str == x.replace(/^eNewlines/,'').toLowerCase()) {
Could use RegExp.rightContext here.

Also, space between if and (

>+          catch(ex) {} // we'll get a security error in non-chrome xul
Then they shouldn't set the attribute...

I did a quick check for upper case attributes values, and only found two; one used in Mail menus and one used in the Suite component bar (Mail component!); we have quite a lot of mixed-case attribute names and ids (which I suppose are a special case of attribute values).
Attachment #202039 - Flags: superreview?(jag) → superreview+
Comment on attachment 202040 [details] [diff] [review]
reworked toolkit binding patch

I like this approach, except for having to enumerate all the properties on nsIPlainttextEditor. I don't see a better approach though.

And Neil's right, the security exception in non-chrome XUL will be more informative than silently failing.

One more nit:

+          this.inputField.value=str;

Put spaces around =

Attach a new patch with this and Neil's comments addressed, for both toolkit and xpfe, and I think we've got a winner.
I'm not sure if not catching an exception in an XBL constructor will break things in XUL.  I'll check that out before I update the patch.
Attached patch toolkit patch (obsolete) — Splinter Review
Done and done
Attachment #202040 - Attachment is obsolete: true
Attachment #202621 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #202040 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch xpfe patch (obsolete) — Splinter Review
Same thing, xpfe
Attachment #202622 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch toolkit patch (obsolete) — Splinter Review
Changed to use RegExp.test
Attachment #202621 - Attachment is obsolete: true
Attachment #203133 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #202621 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch xpfe patch (obsolete) — Splinter Review
same for xpfe
Attachment #202622 - Attachment is obsolete: true
Attachment #203134 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #202622 - Flags: review?(neil.parkwaycc.co.uk)
(In reply to comment #25)
>I'm assuming that everyone who needs nsIDOMKeyEvent.h is already including it).
Everyone except mozInlineSpellChecker.cpp :-/
Two more issues:
1. You changed the value of some of the enums without updating anyone's prefs
2. I assume you are requiring the newlines attribute value to be in lower case?
Oh, and which fields are you proposing to affect? For starters, a quick scan of LXR turns up the following URL autocomplete fields: editor link properties; DOM Inspector; Chatzilla prefs; URLbar; open location dialog; home page pref; email addressing; address lists; mail start page pref; attach page dialog.
(In reply to comment #41)
> Two more issues:
> 1. You changed the value of some of the enums without updating anyone's prefs

Ouch, good catch.  I guess for compatibility's sake we need to switch back to the previous values.  Maybe file a followup bug on removing use of the pref from the tree?

> 2. I assume you are requiring the newlines attribute value to be in lower case?

Yes.

(In reply to comment #42)
> Oh, and which fields are you proposing to affect? For starters, a quick scan of
> LXR turns up the following URL autocomplete fields: editor link properties; DOM
> Inspector; Chatzilla prefs; URLbar; open location dialog; home page pref; email
> addressing; address lists; mail start page pref; attach page dialog.
> 

The primary target would be the URLbar, but any textbox expecting URL input is a fair target.  I would expect one or more followup bugs to use the pref anywhere it's appropriate.  If we leave the enum values unchanged from the originals, we shouldn't break any existing pref'ed behavior.(In reply to comment #40)

> (In reply to comment #25)
> >I'm assuming that everyone who needs nsIDOMKeyEvent.h is already including it).
> Everyone except mozInlineSpellChecker.cpp :-/

Do we need to undo that IDL change, or just fix that separately?


Depends on: 319882
Attached patch editor patchSplinter Review
Ok.  Restored numbering of the options back to the originals, so as not to break existing pref users.  Added an include of nsIDOMKeyEvent.h in mozInlineSpellCheck.cpp to avoid breaking spellcheck.
Attachment #202039 - Attachment is obsolete: true
Attachment #205562 - Flags: superreview+
Attachment #205562 - Flags: review?(neil.parkwaycc.co.uk)
Attached patch Updated toolkit patch (obsolete) — Splinter Review
Updating toolkit patch due to changes in bug 302050
Attachment #203133 - Attachment is obsolete: true
Attachment #205563 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #203133 - Flags: review?(neil.parkwaycc.co.uk)
So, I tried this, and it works great in the URLbar; I then tried it in the Compose addressing widget, but that doesn't work so well; editor is null :-(
(In reply to comment #46)
>So, I tried this, and it works great in the URLbar; I then tried it in the
>Compose addressing widget, but that doesn't work so well; editor is null :-(
Sigh. Compose is just being weird. Something to do with it cloning nodes.
Attachment #203134 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 205562 [details] [diff] [review]
editor patch

r=me on the changes from attachment 202039 [details] [diff] [review].
Attachment #205562 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 205563 [details] [diff] [review]
Updated toolkit patch

>-        var str = this.boxObject.getProperty("value");
>+        var str = this.boxObject.getProperty("value"); 
Oops, you dropped a space in at the end of this line. (I hope your xpfe patch doesn't have any lines with trailing spaces, I didn't think to check)

>         }
>         str = this.getAttribute("spellcheck");
...
>         }
>+        str = this.getAttribute("newlines");
I'd probably put blank lines between your sections. And I'd put your code before spellcheck, it's obviously more important ;-)
Attachment #205563 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #205563 - Flags: superreview+
Ted: do address Neil's comments before checking in, of course

Neil: So editor being null in Composer means that we currently shouldn't put this attribute on those editors?
Attached patch Toolkit patchSplinter Review
Updated per Neil's comments.  Also cleans up some fallout from bug 302050 which resulted in two constructor and destructor elements in the textbox binding.
Attachment #205563 - Attachment is obsolete: true
Attachment #206224 - Flags: superreview+
Attachment #206224 - Flags: review+
Attached patch xpfe patchSplinter Review
For pedantry's sake, I removed that trailing space from the one line in the xpfe patch.
Attachment #203134 - Attachment is obsolete: true
Attachment #206225 - Flags: superreview+
Attachment #206225 - Flags: review+
Checked in by timeless.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Depends on: 321238
Hm, we could've trimmed a couple lines of code from these patches if I'd implemented the editor property of textbox.xml in bug 312867.  Sorry I didn't notice.
My debug build of Firefox on Windows throws an exception on this bit of code:

+                this.inputField.QueryInterface(nsIDOMNSEditableElement)
+                    .editor.QueryInterface(nsIPlaintextEditor)
+                    .newlineHandling = nsIPlaintextEditor[x];

It seems that the QI to nsIPlaintextEditor fails, and as a result the XBL binding is not properly initialized.  This causes the URL bar to be blank on startup, and after link clicks, etc.

The exception:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: chrome://global/content/bindings/textbox.xml ::  :: line 126"  data: no]

My tree seems to be pretty clean, so I don't know yet how to explain this.  Is anyone else seeing this?
> My debug build of Firefox on Windows throws an exception on this bit of code:

False alarm.  I believe this was due to a depend build problem on Windows :-(
I'm still getting that error here and have a blank URLbar with a clean debug build (but not with a release build). Bug 321238 was the original one for the issue, although the asserts seem to have gone now, but the JS exception you mentioned is still occuring here.

And if Neil mentioned build dependancy problems I will hurt him. This is a clean build ffs.
Blocks: 329668
Attached patch mochichrome testSplinter Review
Better late than never!
Attachment #279994 - Flags: review?(enndeakin)
Attachment #279994 - Flags: review?(enndeakin) → review+
Comment on attachment 279994 [details] [diff] [review]
mochichrome test

Not sure if tests need approval, but asking anyway.
Attachment #279994 - Flags: approval1.9?
Comment on attachment 279994 [details] [diff] [review]
mochichrome test

mconnor said tests don't need approval.  Checked in.
Attachment #279994 - Flags: approval1.9?
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.