Open Bug 552782 Opened 10 years ago Updated 8 years ago

Outlines: Numbering is wrong

Categories

(Core :: Editor, defect)

defect
Not set

Tracking

()

People

(Reporter: ned.phipps, Unassigned)

Details

(Whiteboard: [only test landed, don't close on inbound merge])

Attachments

(1 file)

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; GTB6.4; .NET CLR 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30; .NET CLR 3.0.4506.2152; .NET CLR 3.5.30729)
Build Identifier: 

Outline numbers are wrong if outdent then indent.

Reproducible: Always

Steps to Reproduce:
1.Start an outline
2.Make three sub-items in a set
3.Outdent item 2 so its at the main level
4.Indent item 2 so its back where it was

Actual Results:  
***Item 2 has the number 1 instead of 2

Expected Results:  
The numbers should be the same as before step 3

The software leaves this above the item 2 in the file:
</ol>
<ol>
Component: General → Composer
Version: unspecified → SeaMonkey 2.0 Branch
This is probably an issue with core than with SM composer
Status: UNCONFIRMED → NEW
Component: Composer → Editor
Ever confirmed: true
OS: Windows XP → All
Product: SeaMonkey → Core
QA Contact: general → editor
Hardware: x86 → All
Version: SeaMonkey 2.0 Branch → Trunk
Actually what I see on current trunk, after outdenting and then indenting "Item 2" is:
  1. Item 1

     Item 2

  1. Item 3

Would we expect it to be:
  1. Item 1
  2. Item 2
  3. Item 3
?

If starting with something indented more and then outdenting item 2 once, would we expect it to be:
    1. Item 1
  1. Item 2
    1. Item 3
but it is:
    1. Item 1

Item 2

    1. Item 3

Then reindenting after, gives:
    1. Item 1

  Item 2

    1. Item 3
But would expect:
    1. Item 1
    2. Item 2
    3. Item 3

Reindenting "Item 2" from:
    1. Item 1
  1. Item 2
    1. Item 3
Gives:
    1. Item 1
    1. Item 2
    2. Item 3
But would expect:
    1. Item 1
    2. Item 2
    3. Item 3
Hmm, this is interesting...  I understand the desired opposite behavior of indent and outdent, but I'm not sure if it makes sense to put item 2 back in the list or not...
I would say that is wrong behaviour for a single outdent of one item in a twice indented list to remove all indents for that item though. The question is, should it be:
      1. Item 1
   1. Item 2
      1. Item 3
or:
      1. Item 1

   Item 2

      1. Item 3
or something else?
This is all a huge, huge complicated mess.  (Moreso than most of editing.)  I spent a long time researching what various UAs do and thinking about it, and the results are summarized in the giant comment here:

http://aryeh.name/spec/editing/editing.html#toggling-lists

My conclusion was that the list marker should always be one plus the list marker of the previous item of the same indentation level, assuming nothing of lower indentation level intervenes.  The upshot of that is we should never produce </ol><ol>.  The lists should always be merged in that case.  Thus if you have

  <ol>
    <li>foo</li>
    <ol>
      <li>bar</li>
      <li>baz</li>
      <li>quz</li>
    </ol>
  </ol>

and you outdent "baz", it should become

  <ol>
    <li>foo</li>
    <ol><li>bar</li></ol>
    <li>baz</li>
    <ol><li>quz</li></ol>
    </ol>
  </ol>

Then if you indent "baz", it should join with the existing two lists and (in this case) you should get the original markup back.  Having two lists that are adjacent but have different numbering doesn't make sense for WYSIWYG, IMO, unless the user manually sets the numbers somehow (<li value>).

The exact details of toggling lists are one of the most complicated parts of the spec, though.
The steps should never generate two lists.

Step 1 -- create:
   1. A
      1. B1
      2. B2
      3. B3

Step 2 -- outdent 
   1. A
      1. B1
   2. B2
      1. B3

Step 3 -- re-indent should give
   1. A
      1. B1
      2. B2
      3. B3

Instead it gives:
   1. A
      1. B1
      1. B2
      2. B3
Attached patch TestSplinter Review
Comment on attachment 561020 [details] [diff] [review]
Test

Test for outdenting/indenting issue described in comment 6
Attachment #561020 - Flags: review?(ehsan)
Comment on attachment 561020 [details] [diff] [review]
Test

Ian, if you have run this through the try server, we can land it.
Attachment #561020 - Flags: review?(ehsan) → review+
Try run for 6886b03f63e8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6886b03f63e8
Results (out of 31 total builds):
    success: 30
    warnings: 1
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/iann_cvs@blueyonder.co.uk-6886b03f63e8
Keywords: checkin-needed
Assignee: nobody → iann_bugzilla
https://hg.mozilla.org/integration/mozilla-inbound/rev/1384ec5bb542
Assignee: iann_bugzilla → nobody
Flags: in-testsuite+
Keywords: checkin-needed
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla9
This bug is not fixed by Ian's patch, and I don't know if he's volunteered to write the fix in addition to writing the test, so let's not assign it to him or set the target milestone, etc.  in-testsuite+ is the only thing this check-in needs.
Assignee: iann_bugzilla → nobody
Target Milestone: mozilla9 → ---
Oh |todo_is|. 

My skim read whilst preparing this for checkin-needed (before you beat me to it hehe) thought this was a WFM since originally reported, but a test added to make sure it didn't regress.
Whiteboard: [only test landed, don't close on inbound merge]
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.