Closed Bug 200416 Opened 22 years ago Closed 12 years ago

deleting newline causes content to change order

Categories

(Core :: DOM: Editor, defect, P2)

Product:
Core
Component:
DOM: Editor
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.5beta

People

(Reporter: barrowma, Assigned: ayg)

References

(Depends on 1 open bug)

Details

(Keywords: topembed+, Whiteboard: EDITORBASE+, edt_b3, edt_c3, edt_x3)

Attachments

(3 files, 10 obsolete files)

sample showing weird deleting behavior
545 bytes, text/html
Details
Patch v13, passes tests
20.37 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review
Test
1.64 KB, patch
ehsan.akhgari
: review+
Details | Diff | Splinter Review 
1. Open the attachment in the editor.  
2. Place the caret at the start of the 2nd line of text (to the immediate left
of the 'I' in "Intro").
3. Hit Backspace

Expected: I expect the newline to go away and the word "Intro" to be moved to
the end of the previous line.

Observed: The word "Intro" jumps to the 1st line, pushing the 1st line down.  

I am using trunk build from 4/2/2003, id: 2003040209.

    
  
Whiteboard: EDITORBASE

    
    

    
  
Possibly related to bug 192320.

    
    

    
  
I can reproduce on Linux build from 03/29/2003, so apparently NOT related to the
fix for bug 192320.

    
    

    
  
Confirming, this is not caused by that other checkin.
I can reproduce this bug even with Mozilla 1.0

    
    

    
  
-> me
Assignee: jfrancis → kaie

    
  
Keywords: topembed → topembed+
Whiteboard: EDITORBASE → EDITORBASE, edt_b3, edt_c3, edt_x3

    
    

    
  
This is caused by having a block level element (<p>) inside an inline (<font>).
 This situation leads to strange behavior from the editor, because it tries to
satisfy the strict dtd for all editor actions.

Allowing the editor to break those rules will lead to undesirable side effects.

    
  
Depends on: 202437

    
    

    
  
EDITORBASE+. Need to investigate common cases of invalid markup causing problems.
Priority: -- → P2
Whiteboard: EDITORBASE, edt_b3, edt_c3, edt_x3 → EDITORBASE+, edt_b3, edt_c3, edt_x3

    
    

    
  
Kai, this looks related to bug 141019 which I posted a patch for back in 09/2002
but we held off on it because we were hoping to tackle the problem of handling
block-in-inline situations during reflow.

You might want to see if the patch in that bug fixes this problem too?

    
    

    
  
Kin, unfortunately the patch from bug 141019 does not fix this bug, and in
general doesn't seem to work anymore. See also the comment I added there.


    
    

    
  
It seems the issue is simpler than we thought, I think I found a very general
bug in the JoinBlocks code, if one block contains the other.

The code calls a helper function IsDescendantOf. That function returns the
offset (position) of the input node into a parent node.

However, the caller makes the incorrect assumption this offset will refer to the
input parent node, but in this bug the offset actually refers to a node between
the input parent node and the input test node.

The bug is, the caller makes the incorrect assumption and uses the offset with
the original input parent node. This is an incorrect position.

The solution is to extend the helper function IsDescendantOf, to make it return
the complete pair that is required to indicate the found position, and change
the caller to use that correct position instead.


    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1 (obsolete)
        — 
          — Splinter Review
      

    


  
Suggested fix.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v1 (obsolete)
        — 
          — Splinter Review
      

    


  
Same patch, larger context in diff.

Note that while I changed the parameter list of the function, I removed
parameters from two callers that are obviously not using the output data, and
by not passing in the out parameter pointer, we save some cycles.

        Attachment #122161 -
        Attachment is obsolete: true

    
  

        Attachment #122162 -
        Flags: review?(jfrancis)

    
  

        Attachment #122162 -
        Attachment is obsolete: true

        Attachment #122162 -
        Flags: review?(jfrancis)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v2 (obsolete)
        — 
          — Splinter Review
      

    


  
Updated patch

    
  

        Attachment #122177 -
        Flags: review?(jfrancis)

    
    

    
  
are we trying to get this in for 1.4final or 1.5?  lets update target milestone..

    
  

        Attachment #122177 -
        Flags: review?(jfrancis)

    
    

    
  
Setting target milestone to 1.4 beta, because I don't see a 1.4 final milestone.

Target Milestone: --- → mozilla1.4beta

    
    

    
  
Comment on attachment 122177 [details] [diff] [review]
Patch v2

This patch is wrong, thanks to Joe for pointing that out.
It only worked caused I was lucky.
My success during testing stopped me from reviewing my own code too early.

        Attachment #122177 -
        Attachment is obsolete: true

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Experimental Patch v3 (obsolete)
        — 
          — Splinter Review
      

    


  
This code is what I had originally intended.

It is different from what Joe suggested yesterday (on IRC), but I would like to
understand, whether this might still make sense, and if not, why.


Let me illustrate what we have and the strategy of this experimental patch:

We have

<body>		  <-- left block
  <inline>
    text 1
    <block>	  <-- right block
      text 2

The code tries to move "right block" to a different place.

When the code looks for the destination position of the move, it will always
use a position that uses an offset in combination with the "left block", which
is "<top>" in this case.

I believe this strategy is wrong. The current code ignores the fact that there
can be more than one nesting level between "left block" and "right block".

It appears more logically to me, to simply move the "right block" exactly one
level up, instead of moving it directly below the "left block".

This patch will use the position of "right block" as the destination position
for the new content, the idea is to produce:

<body>
  <inline>
    text 1
    text 2

It seems to work. Does that code make sense?

    
    

    
  
Comment on attachment 122580 [details] [diff] [review]
Experimental Patch v3

Joe, what do you think about this approach?

        Attachment #122580 -
        Flags: review?(jfrancis)

    
    

    
  
When I said 
  simply move the "right block" exactly one level up
I of course meant
  move the CONTENTS of "right block" exactly one level up


    
    

    
  
Kai, the strategy you discuss is more elegant from a coding viewpoint than the
strategy we use now, but in my experience it does not correspond with user
expectations.  The reason for this is that users perceive documents
"horizontally".  When you type backspace, they expect a block to be joined to
what came "before", rather than what came "above".  Example:

<body>
  <div>
    <h1> Section One </h1>
    Here is the text to section one of my document.
  </div>
  <div>
    <h1> Section Two </h1>
    Here is the text to section two of my document.
  </div>
</body>

Consider putting the caret at the front of: <h1> Section Two </h1>
and typeing backspace.  Under your plan we would get rid of the heading, but not
move the "Section Two" text out of it's current div.  But users expect the
backspace to join that text to the earlier text: the text in the first div.

The editor used to work more like the way you propose.  Since I changed it, I
have received far fewer complaints about how blocks behave when backspacing or
forward-deleting over block boundaries, so I think we should stick with the
strategy we have now.  That strategy is to try to join blocks with what came
before, rather than what came above (of course, sometimes those two are the same).

    
  

        Attachment #122580 -
        Attachment is obsolete: true

        Attachment #122580 -
        Flags: review?(jfrancis)

    
    

    
  
Joe, I agree with your comments.


    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v5 (obsolete)
        — 
          — Splinter Review
      

    


  
This patch looks scary, but it isn't.
Most of the patch is a change that doesn't affect the logic at all.
I'm moving the "obtain parent block" into the JoinBlocks method, and because
the variable is no longer a pointer, a lot of code changes from "*variable" to
"variable".

This is necessary, because inside the JoinBlocks method, we need access to the
original node, not only to its parent.



There is only a single place in the patch that contains the real change.
Search for
  @@ -2487,7 +2476,27 @@
It contains a comment and the new logic.

    
  

        Attachment #122698 -
        Flags: review?(jfrancis)

    
  
Target Milestone: mozilla1.4beta → mozilla1.4final

    
  
QA Contact: sairuh → beppe

    
    

    
  
Comment on attachment 122698 [details] [diff] [review]
Patch v5

This looks good.  Have you tested it with a case in which there are multiple
levels of inlines between the blocks?  I don't forsee a problem there but it
would be good to make sure it all works as expected.

        Attachment #122698 -
        Flags: review?(jfrancis) → review+

    
    

    
  
Joe,

> Have you tested it with a case in which there are multiple
> levels of inlines between the blocks?

the original reported case was
  <font>Text1<p><font>Text2></font><p></font>

and now I tested
  <big><font>Text1<p><font><small><font>Text2
  </font></small></font></p></font></big>

Both appear to behave correctly.

    
  

        Attachment #122698 -
        Flags: superreview?(sfraser)

        Attachment #122698 -
        Flags: superreview?(sfraser) → superreview+
OS: Windows XP → All
Hardware: PC → All

    
    

    
  
Charles found a crash that is seen with this patch and "testcase 2" in bug 202166.

The problem is: It is possible to use edit actions to place the caret at an
invalid position. See bug 202166 comment 5.

When you are placed at an invalid position, and you hit DEL, the code in the
patch in this bug tries to split the style above the point. However, when the
caret is at the invalid position, the "point above" might be the top HTML node,
and the DOM code will abort when an attempt is made to insert additional nodes
to do the split.

I wonder which of the following we should do:
1) add an updated patch to this bug to add some self protection
2) add self protection code to SplitStyleAbovePoint,
   and make it return "success", to make callers continue their execution
3) add self protection code to SplitStyleAbovePoint,
   and make it return "failure", to make callers stop their execution
4) make sure bug 202166 gets fixed first, and make sure no action
   can place the caret on the invalid position - although this might
   be tricky to guarantee


    
    

    
  
Instead of the call to SplitStyleAbovePoint, I used the following fragment,
which makes the crash with the testcase 2 in bug 202166 go away.

      if (!nsTextEditUtils::NodeIsType(p, NS_LITERAL_STRING("html")))
      {
        res = mHTMLEditor->SplitStyleAbovePoint(address_of(p), &offset, 
                                                nsnull, nsnull, nsnull, nsnull);
        if (NS_FAILED(res)) return res;
      }


    
    

    
  
The crash is unrelated to 202166.  When joining blocks, one of the blocks may be
the body.  We should never try to split style above the body, so something like
the check in your prior comment is needed (there is an IsRootNode() helper you
can use to avoid having to declare a string).

One thing I don't understand is why the split is being done above aLeftNode in
this case (when leftBlock contqains rightBlock).  Shouldn't we be splitting
above aRightBlock instead?

Also, I don't understand why SplitStyleAbovePoint() was ever splitting the body
node at all.  It should only be splitting inline style nodes.  There is
something else wrong here...

    
    

    
  
Although we have a reviewed patch, I'd like to move this patch to 1.5 alpha,
because of the risks we are seeing.
Target Milestone: mozilla1.4final → mozilla1.5alpha

    
    

    
  
Comment on attachment 122698 [details] [diff] [review]
Patch v5

Marking as obsolete until we know the patch is really correct. I fear it's not.

        Attachment #122698 -
        Attachment is obsolete: true

    
    

    
  
After digging into the meaning of SplitStylesAbovePoint more, after more
thinking and testing, I think the current approach is not yet correct, and I'm
even somewhat confused.

Joe, you said, when backspacing between content, we need to split the styles,
because we don't want the content below the caret to receive the style of the
content above the caret. Are you sure? What about this:

  <font style="color: red">
    left red<br>
    <p>
      right, no style
    </p>
  </font>

The text "right, no style" is displayed in red.
If we split the style prior to moving, we'd get (well, not with the current patch):

  <font style="color: red">
    left red<br>
  </font>
  right, no style
  <font style="color: red">
  </font>

Is that what you suggested? In this case, the second line is no longer displayed
in red! I think that's not what I'd expect.


    
    

    
  
I'm more confused.

<font style="color: red; background-color: rgb(0, 255, 255);">
  left red<br>
  <p>
    right, no style
  </p>
</font>

In this example, both strings are displayed with a color of red, but only the
first string has the defined background color.

Is that a side effect of the invalid HTML (the <p> inside <font>) ?

If we can't predict what happens to styles with that invalid HTML (and that is
what this bug is about), does it make sense at all to try to care for styles?


    
    

    
  
I have no problem with the paragraph losing the style here.  I think getting the
block legally contained in another block is more important.  Then future editing
on that block (including setting the style, if the user so desires) will be more
likely to work correctly.

    
    

    
  
Joe, if I understand correctly, you say you're ok with a patch that is identical
to v5, with the only difference of having the call to SplitStyleAbovePoint
removed. I'll attach that patch tomorrow.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v7 (obsolete)
        — 
          — Splinter Review
      

    


  
This patch is nearly identical to v5, the single change is: Removed the call to
SplitStyleAbovePoint.

    
  

        Attachment #124515 -
        Flags: review?(jfrancis)

    
    

    
  
Comment on attachment 124515 [details] [diff] [review]
Patch v7

I found another case that makes me attach yet another revision of this patch.

Load attachment 123082 [details] from bug 202166 in the editor.
Hit DEL twice.
The caret is now at an invalid position.

When you now hit DEL again, the existing code (without the patch) magically
fixes the situation. With the patch, we cause more breakage and actually
produce text placed after the body close tag.

So I looked what happens, when you hit DEL the third time, without having
applied the patch.

In the call to JoinBlocks, the LeftNode parameter is the root element!

I traced more, and we actually end up at the same MoveBlock call that I'm
changing. But for some reason, the existing code in MoveBlock seems to be able
to deal with this special situation correctly. I have not tried to understand
why.

Because of that, I'm wrapping my new code with a test against IsRootNode. Only
if it's not the root node, we will try to use the new logic.

        Attachment #124515 -
        Attachment is obsolete: true

        Attachment #124515 -
        Flags: review?(jfrancis)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v8 (obsolete)
        — 
          — Splinter Review
      

    


  
Same as before, added:
+      if (mHTMLEditor->IsRootNode(aLeftNode))
+      {
+	 // Continue to use the old style logic, that magically works correctly

+	 // when the caret is placed at an invalid position,
+	 // indicated by aLeftNode being the root node...
+	 res = MoveBlock(aLeftBlock, aRightBlock, leftOffset, rightOffset);
+      }
+      else

    
  

        Attachment #124523 -
        Flags: review?(jfrancis)

    
    

    
  
I don't think this is going to work.  I will prepare a testcase to illustrate
the problem, but first I want to go look at yor other review requests and knock
off any easy ones.

    
    

    
  
A couple of comments:

1)
Instead of checking to see if aLeftNode is the root node, check to see if
aLeftNode==leftBlock, and if so use leftBlock as the param to MoveBlock()
instead of parent of aLeftNode.  This will prvent incorrect results if we ever
get aLeftNode==leftBlock, but not the root node.

2)
We have to split the style.  Otherwise if you have this:
<p><b>bold para</b></p>
<p>unbold para</p>
when you backspace at front of second p, it will become bold.  Style should not
change when blocks are merged unless the style is part of the nature of the
block (like font size and weight in headings).  For an even worse example you an
<a> instead of a <b>.

So I still think we should follow my orginal idea of splitting any inline style
at the spot we are moving rightBlock to, and then moving rightBlock into
LeftBlock at the place where the style splitting terminated in leftBlock.

    
  

        Attachment #124523 -
        Flags: review?(jfrancis) → review-

    
    

    
  
> We have to split the style.  Otherwise if you have this:
> <p><b>bold para</b></p>
> <p>unbold para</p>
> when you backspace at front of second p, it will become bold.

No, it does not become bold. :-)
I just tested the patch with both examples (b and a).

The separate styles continue to be separate.


    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v10 (obsolete)
        — 
          — Splinter Review
      

    


  
> 1)
> Instead of checking to see if aLeftNode is the root node, check to see if
> aLeftNode==leftBlock, and if so use leftBlock as the param to MoveBlock()
> instead of parent of aLeftNode.  This will prvent incorrect results if we
ever
> get aLeftNode==leftBlock, but not the root node.

This suggestion works pretty well, thanks.
Attaching new patch with this change only.

        Attachment #124523 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 126499 [details] [diff] [review]
Patch v10

I am re-requesting review, because I do not see the problem you mention.

The second block does not acquire the style of the first block.

It seems the call to SplitStyle is not necessary, unless you know a
reproducable case that shows unwanted style merging.

        Attachment #126499 -
        Flags: review?(jfrancis)

    
    

    
  
Comment on attachment 126499 [details] [diff] [review]
Patch v10

in non-css mode editor, try this source:
<body background="">
<b>line1</b>
<p>line2</p>
</body>

In css mode editor, try this source:
<body>
<span style="font-weight: bold;">line 1</span>
<p>line 2</p>
</body>

In either case backspacing from front of line2 causes line2 to become bold.

        Attachment #126499 -
        Flags: review?(jfrancis) → review-

    
    

    
  
I can't put into words how much your comment confuses me.
Ironically said, I have the impression we are talking about different Mozilla
projects!

I have tried:
- mozilla 1.4 in css mode
- mozilla 1.4 in non-css mode
- mozilla latest trunk in css mode
- mozilla latest trunk in non-css mode

I have tried each of the above with both examples given in comment 42.
That means, I have tried 8 combinations.

I did not see any style merging in any combination.
Text "line 1" always remained bold, and text "line2" always remained non-bold.

I have one last idea. Maybe this is a platform dependent problem?
I did all my tests on Linux.
I can try on Windows or Mac, maybe that will behave differently.

    
    

    
  
Mozilla 1.4, Mac OS X.

Again, "line 2" does not get bold when backspacing to join the lines.

    
    

    
  
I have now tested the 8 combinations on Windows 2000, 1.4/trunk, css/non-css,
testcase/testcase2, and still I don't see what you see. 

    
  
Target Milestone: mozilla1.5alpha → mozilla1.5beta

    
    

    
  
Don't ask me why it didn't work earlier, but now I finally see what you report!

With the patch applied, the second line turns bold on backspace.

Thanks to Joe and Kathy for review comments and testing.


    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v11 (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #126499 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 127407 [details] [diff] [review]
Patch v11

As usual, I think the new patch handles all cases described above correctly :-)
Please review.

        Attachment #127407 -
        Flags: review?(jfrancis)

    
    

    
  
It's not likely that I will work on editor/selection bugs in the near future.
Mass assining my bugs to nobody.


Assignee: kaie → nobody

    
    

    
  
snarfing kaie's old bugs
Assignee: nobody → mozeditor

    
    

    
  
snarfing kaie's old bugs

    
  
Assignee: mozeditor → kaie

    
  
Assignee: kaie → mozeditor
QA Contact: rubydoo123 → bugzilla

    
    

    
  
Comment on attachment 127407 [details] [diff] [review]
Patch v11

brade: could you review this?

        Attachment #127407 -
        Flags: review?(mozeditor) → review?(brade)

    
    

    
  
Comment on attachment 127407 [details] [diff] [review]
Patch v11

+	 // We try to work as good as possible with HTML that's already
invalid.

The above line should change "good" to "well".


note:  the only difference between patch 11 and patch 5 which had r/sr=
(besides offset numbers) is in this block @@ -2626,7 +2653,57 @@.  (@@ -2487,7
+2476,27 @@ in patch v5)

I haven't done any testing with this patch (at least not in 9+ months).

r=brade (although I wish Joe or Kin could look at this too)

        Attachment #127407 -
        Flags: superreview?(kinmoz)

    
    

    
  
Don't count my r= from patch 5, it had problems.  Before getting an r+, my
testcase in comment 42 must be passed.

    
  
QA Contact: bugzilla → editor

    
  
Assignee: mozeditor → nobody

    
    

    
  
Ehsan, any value in this bug still?

    
    

    
  
Sigh... Yeah we still have the bug but the patch is obviously rotten throughout the years.  Aryeh, can you see if you can revive the patch?  I think the basic idea of the patch should still apply.

    
  

        Attachment #127407 -
        Flags: superreview?(kinmoz)

        Attachment #127407 -
        Flags: review?(brade)

    
    

    
  
Patch seems to apply okay after some fixups -- shows you how much attention this code has seen!  Also seems to achieve the desired effect.  It just remains to be seen whether it causes any test failures.
Assignee: nobody → ayg
Status: NEW → ASSIGNED

    
    

    
  


  
I ported to current tip and fixed style issues on touched lines.  This basically works, but it causes a whole bunch of failures of exactly the sort it seeks to prevent, e.g.,

218 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/imptests/editing/conformancetest/test_runtest.html | [["defaultparagraphseparator","div"],["delete",""]] "foo[<div><p>]bar</p>baz</div>" compare innerHTML; assert_equals: Unexpected innerHTML (after normalizing inline style) expected "foobar<div>baz</div>" but got "foo<div>baz</div>bar"

The spec has a (I think!) good algorithm here, namely repeatedly "split the parent" of the nodes to move until their parent is correct.  The "split the parent" algorithm depends fairly heavily on other parts of the spec, though, so it's not so practical to import.  Let me see if I can figure out what's going on.

        Attachment #127407 -
        Attachment is obsolete: true

    
    

    
  


  
There we go -- there was a slight bug in the patch that I fixed.  I admit I don't fully understand it, but it seems to have the desired effect, and also passes tests.  The minimal test-case to reproduce is:

  <span>foo<p>bar</p></span>

Place the cursor before "bar" and hit delete.  I'll add this test to the upstream editing tests and submit a separate patch to sync, so we have test coverage -- this patch by itself has no regression test.

Try: https://tbpl.mozilla.org/?tree=Try&rev=c0184a42d035

When reviewing, it helps to realize that the only functional changes should be a) moving some code from two callers of JoinBlocks into the beginning of the function, and b) adding some extra special-casing near the end of JoinBlocks.  Everything else is just because of

-nsHTMLEditRules::JoinBlocks(nsCOMPtr<nsIDOMNode> *aLeftBlock,
-                            nsCOMPtr<nsIDOMNode> *aRightBlock,
+nsHTMLEditRules::JoinBlocks(nsIDOMNode *aLeftNode,
+                            nsIDOMNode *aRightNode,

which necessitates the removal of a lot of *'s and the addition of some address_of()s.  If there are any functional changes there, it's probably a bug.  (If I were writing the patch from scratch, I'd have separated that out into a separate patch, but it's probably easier for you to just review it as-is than for me to separate it out at this point.)

        Attachment #684943 -
        Attachment is obsolete: true

        Attachment #684944 -
        Flags: review?(ehsan)

    
    

    
  
So it looks like the spec is broken for this specific test-case, too, so I can't add a usable test upstream (since the expected result would be wrong).  I'll write a separate mochitest.

    
    

    
  
Comment on attachment 684944 [details] [diff] [review]
Patch v13, passes tests

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

::: editor/libeditor/html/nsHTMLEditRules.cpp
@@ +2503,5 @@
>                              bool *aCanceled)
>  {
> +  NS_ENSURE_ARG_POINTER(aLeftNode && aRightNode);
> +
> +  nsCOMPtr<nsIDOMNode> aLeftBlock, aRightBlock;

I'd prefer that we don't use the 'a' prefix for non-arguments.  But I can do that myself when landing your patch.

        Attachment #684944 -
        Flags: review?(ehsan) → review+

    
    

    
  
(In reply to :Aryeh Gregor from comment #60)
> So it looks like the spec is broken for this specific test-case, too, so I
> can't add a usable test upstream (since the expected result would be wrong).
> I'll write a separate mochitest.

OK, so let's wait for the test before landing this then.  Thanks!

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Test
        — 
          — Splinter Review
      

    


  
The exact innerHTML we produce here is still garbage, although in this case it looks okay:
  <span>foo</span>bar<span></span>
It most likely should be:
  <span>foobar</span>
But the spec is even worse:
  <span>foo<br></span>bar
No idea how the <br> gets there.  Should I file a follow-up on fixing the exact HTML, pending a spec change (which I don't expect to make in the foreseeable future)?

        Attachment #688213 -
        Flags: review?(ehsan)

    
    

    
  
Yes, please.

    
  

        Attachment #688213 -
        Flags: review?(ehsan) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/8b86519c08a0
https://hg.mozilla.org/mozilla-central/rev/5d9989bb4d2f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: