233261 - DL/DT/DD polish

Status: RESOLVED FIXED

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

Description

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

When one selects Format > List > Definition Term|Definition, the CR key
does not create the corresponding Definition|Term. That's *very* annoying
and a major user feedback.

Comment 1

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Attachment: fix #1

Comment 2

[Kathleen :Brade]

Comment on attachment 140743 [details] [diff] [review]
fix #1

Do you need to call CopyLastEditableChildStyles when creating a new dd or dt? 
If I was typing with a green text color (or a particular font or whatever) I'd
expect that to continue (or is that happening already)?

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Kathy: when I create a new element but switch the element type, from dd to dt or
vice-versa, I don't need that because the semantics of dt and dd are different.
It makes no sense to preserve a bold style assigned to a dt when you move the caret
to the following dd :-)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 140743 [details] [diff] [review]
fix #1

sr=dbaron, assuming you get owner/peer review, although the following:

>+          nsAutoString listTag((nodeAtom == nsEditProperty::dt) ? NS_LITERAL_STRING("dd") : NS_LITERAL_STRING("dt"));

would perhaps be better as:

nsAutoString listTag;
nodeAtom->ToString(listTag);

Comment 5

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #4)

> >+          nsAutoString listTag((nodeAtom == nsEditProperty::dt) ?
NS_LITERAL_STRING("dd") : NS_LITERAL_STRING("dt"));
> 
> would perhaps be better as:
> 
> nsAutoString listTag;
> nodeAtom->ToString(listTag);

dbaron: no; that line switches dd and dt... your rewriting does not.

Comment 6

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 140743 [details] [diff] [review]
fix #1

requesting approval for 1.7 checkin: this fixes a VERY annoying bug in
Composer; that bug makes it almost impossible to edit a DL/DT/DD list without
dreaming of killing the whole editor team... We had quite a lot of user
feedback about it. The fix is simple and harmless, has no impact outside of
DL/DT/DD anyway. It is implemented in Nvu 0.20 and works just fine.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

checked in