Closed Bug 92278 Opened 23 years ago Closed 23 years ago

RFE: Allow "Change just selected items" in List Properties to restrict changing bullet type to selection

Categories

(SeaMonkey :: Composer, defect, P2)

x86
All

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: hwaara, Assigned: cmanske)

References

(Depends on 1 open bug)

Details

(Whiteboard: EDITORBASE (2 days))

Attachments

(2 files, 4 obsolete files)

I wanted to make a list with different kinds of bullets, depending on the level
of the list. Like this:

* Hi
* o Hi
* o * Hi

So I tried first making a usual bulleted list. Then I selected the two last
items, brought up List Properties using the contextmenu, and clicked the
radiobutton "Change just selected items".   It changed all bulleted items: even
the first one.

So my conclusion is: changing just the selected items in a list is broken.
looked at this with Charley, the menu option selection is not triggering the
change. Assigning to cmanske
Assignee: beppe → cmanske
Priority: -- → P3
Target Milestone: --- → mozilla0.9.4
*** Bug 94678 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Target Milestone: mozilla0.9.4 → mozilla0.9.5
spam composer change
Component: Editor: Core → Editor: Composer
Whiteboard: EDITORBASE (2 days)
This is technically "invalid". 
The radio buttons "Change entire list" and "Change just selected items" are 
inside the "List Type" groupbox and thus only applies to changing the list type, 
not the groupbox below for "Number Style" or "Bullet Style". The latter 
currently sets the "type" attribute on either the <UL> or <OL> element, i.e., 
it applies to the entire list.
But the "type" attributes are valid on individual <LI> elements in the
selection, so I'll interpret this a request to change the UI so the radio 
buttons apply to all settings and allow setting of "type" to either the OL, UL, 
or LI elements accordingly.
Joe: Do we have any list methods to easily do this? Seems like I'd have to 
iterate through the selection to find all relevant LI element within the 
selection, no?

Target Milestone: mozilla0.9.5 → mozilla0.9.6
Charlie, the amount of code that you can steal for this depends on the exact spec 
of what you want to do.  What about sublists in the selection?  partially in the 
selection?  what about lists that are inside other structures (like tables) that 
are themselves inside the selected list items?

If you want to change *any* li in the range, it's pretty easy.  If you want to 
change the same li's that make list would change if you clicked the other list 
type, it's easy.  If you want something else, you will have to spin your own but 
it may not be hard.  For instance, maybe you want only change li's in top level 
list that is selected.  Or only li's that have no non-list related structure 
between them and the "top" selected list.  Both of these wouldn't be too hard.
Yes, I would want to apply the "type" attribute to *any* LI in the selection;
i.e., consider partially selected LIs at beginning and end to be in the selection.
Load balancing
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Whiteboard: EDITORBASE (2 days) → EDITORBASE- (2 days)
Priority: P3 → P2
Whiteboard: EDITORBASE- (2 days) → EDITORBASE (2 days)
Unfortunately, this isn't going to happen in time for 0.9.7
Target Milestone: mozilla0.9.7 → mozilla0.9.9
Charley : it CAN be in for 097 ! Reading your bugmail about it, I made an easy fix
that works fine in my tree ; trying to produce a patch right now. Because of 
my family constraints and time shift, I'll leave you have the reviews and try
to check it in !
Comment on attachment 61260 [details] [diff] [review]
patch v1.0

obsoleted by neil's and brade's reviews
Attachment #61260 - Attachment is obsolete: true
Attached patch patch v1.1 (obsolete) — Splinter Review
integrating Neil's and Brade's comments
Attached patch patch v1.3 (obsolete) — Splinter Review
grumble, forgot one point
Comment on attachment 61265 [details] [diff] [review]
patch v1.2

Did you mean '=' instead of '=='?

+var BulletStyleType == "";
+var originalBulletStyleType == "";


I really dislike the fact that we have HTMLisms in our "nsTextRulesInfo" ...
not your fault ... just venting. :-)

I didn't apply the patch to play with it ... is there any way that the type you
set on the list items and the type of list containing the list item are of
different types? ie bullets with numbered list, etc.
I spotted the same errors as Kin, but after fixing those in my tree,
this fix doesn't seem to work when selection crosses list levels. E.g.:
Starting with:
1. item 1
2. item 2
   1. Subitem 1
   2. Subitem 2
3. item 3
4. item 4
I then selected starting in "ite[m 2" and ending within "Sub]item 1"
[ ] represent selection endpoints.
I changed the bullet style to "A,B,C..." and the list looked like:
1. item 1
B. ite[m 2
   1. Subitem 1
3. item 3
4. item 
E. Sub]item 2

So it mangled the list in this case!
Attached patch patch v.1.4Splinter Review
Fixed the "==" in initialization and a couple of text case errors.
Attachment #61267 - Attachment is obsolete: true
Kin: The UI in the dialog should make it clear to the use what to expect.
Note that if the selection spans across different list types, we use the values
from the first one (anchor - selection is considered "mixed"). Thus when user
clicks OK, they should expect to see entire selection converted to whatever list
type is set in the "List Type" menu. The choices allowed in the "Bullet Style"
menu are always appropriate only for the selected List Type, so the appropriate
bullet/number style should be applied correctly.
So while I like Daniels pushing the "fix" into the rules code, I guess we (Joe?)
has to figure out why it doesn't work for the case I describe above.
This problem is important, since the list mangling might be independent of
changing the bullet type. I brought up the dialog with the same list and 
selection as above, and simply pressed OK without changing anything.
This is what happened:
1. item 1
2. ite[m 2
   1. Subitem 1
3. item 3
4. item 
5. Sub]item 2

So "Subitem 2" was yanked out of the sublist and moved to the outer list even
if the bullet type wasn't changed.


Comment on attachment 61278 [details] [diff] [review]
patch v.1.4

ok, assuming the behavior is what is desired (charlie?) the code changes look
fine.  r=jfrancis
Attachment #61278 - Flags: review+
No, Joe, the behavior isn't fine! The problem I describe in #16 happens without
glazman's patch, so we have a pre-existing problem.
Summary: "Change just selected items" in List Properties is horked → RFE: Allow "Change just selected items" in List Properties to restrict changing bullet type to selection
no, i mean assuming the behavior *with the patch* is what you desired, then the 
code is fine.
But once we solve the preexisting problem, I agree that glazman's suggetion is
ok with me if it's ok with Joe!
JFrancis: Do you have a bug on that issue (see #18 above)
Some dialog layout changes are also necessary -- comming soon.
cmanske writes:
>JFrancis: Do you have a bug on that issue (see #18 above)

It's 98547.  It's not easy to fix.
Adding depends for tracking, though we won't wait for that to check this in.
Depends on: 98547
Target Milestone: mozilla0.9.9 → mozilla0.9.8
Attachment #61311 - Flags: review+
Comment on attachment 61311 [details] [diff] [review]
UI portion: Move radio buttons below both "List Type" and Bullet Type" groups since it will apply to both settings

r=brade
Comment on attachment 61278 [details] [diff] [review]
patch v.1.4

sr=kin@netscape.com
Attachment #61278 - Flags: superreview+
Comment on attachment 61311 [details] [diff] [review]
UI portion: Move radio buttons below both "List Type" and Bullet Type" groups since it will apply to both settings

sr=kin@netscape.com
Attachment #61311 - Flags: superreview+
seems it's in trunk :-))))
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Hakan,  does this work for you now?
Yes, this works now.  However, when I tried to verify this bug, I found another.
See bug Bug 125500.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: