12.88 KB, patch
Joe Francis: review+
|Details | Diff | Splinter Review|
UI portion: Move radio buttons below both "List Type" and Bullet Type" groups since it will apply to both settings
1.14 KB, patch
|Details | Diff | Splinter Review|
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
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.
Target Milestone: mozilla0.9.6 → mozilla0.9.7
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
Created attachment 61261 [details] [diff] [review] patch v1.1 integrating Neil's and Brade's comments
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!
Created attachment 61278 [details] [diff] [review] patch v.1.4 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.
Created 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
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
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 firstname.lastname@example.org
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 email@example.com
Attachment #61311 - Flags: superreview+
seems it's in trunk :-))))
Status: ASSIGNED → RESOLVED
Last Resolved: 16 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
You need to log in before you can comment on or make changes to this bug.