Closed Bug 133735 Opened 22 years ago Closed 22 years ago

Copying from a list <li> always copies the position in the list

Categories

(SeaMonkey :: General, defect)

Product:
SeaMonkey
Component:
General
Platform:
x86
Windows 98
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: valerio, Assigned: mozeditor)

References

(
URL
)

Details

(Whiteboard: EDITORBASE+; fixinhand; need a=[adt2])

Attachments

(1 file, 1 obsolete file)

undoing earlier patch of nsDocumentEncoder.cpp
888 bytes, patch
akkzilla
: review+
kinmoz
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.9+) Gecko/20020325
BuildID:    2002032503

Probably related to Bug 133728 but not a duplicate.

When you copy a part of an item in a list, its position in the list is copied in
the clipboard as well.

It applies to Ordered Lists (including with parameters) and Unordered lists.

Example:

<OL>
<LI> Item 1
<LI> Item 2
<LI> Item 3
</OL>

<UL>
<LI> Item 1
<LI> Item 2
<LI> Item 3
</UL>

<OL TYPE=a START=3>
<LI> Item 1
<LI> Item 2
<LI> Item 3
</OL>


Let's say you try to copy only "tem 2" from the second element in every list.

You should get
tem 2
in all cases

instead:

In the first list you'll get:
   2. tem 2

In the second list you'll get:
# tem 2
Note that if you select the whole line you get:
*  Item 3 
( * instead of # )

In the third list you'll get:
   4. tem 2
This is interesting because you don't get the letter (d), nor its position (2),
but the number related to letter d.

You can make further tests at this URL:
http://www.elfqrin.com/Sample.html#Lists


Reproducible: Always
Steps to Reproduce:
1. Go to any webpage with a list, or create your own
2. Select a part of an item in a list (not from the beginning of the line, or
however excluding the list marker)
3. Copy it in a text editor or somewhere else

Actual Results:  The position of the element in the list is pasted at the
beginning of the pasted text even if you didn't select/copy it.

Expected Results:  should paste only the selected/copied text.

This is could be a feature, not a bug, yet it should apply only when you
select/copy the whole element in a list, not just a part of it.
Confirmed on v.9.9+ build 2002032708 win98
Joe, they shouldn't be getting any list info in the sample case (when only part
of the list item is selected), should they?  I thought that only the text was
copied in that case.

Tanu has done some work recently regarding conversion of ordered lists to
plaintext; cc'ing, but I think there are two issues here: whether the <li> and
<ol> or <ul> should be getting copied to the context (Joe), and the issue about
ol order (Tanu).
Akkana is correct that this behavior is a bug.  It is likely a document encoder 
bug.  I am the mamajama (doh!).
Assignee: asa → jfrancis
Target Milestone: --- → mozilla1.0.1
*** Bug 134046 has been marked as a duplicate of this bug. ***
Ya, document encoder has been altered in a bad way.  Someone added "li" to 
IncludeInContext().  This is very bad, and casues this bug.  Tanu, did you do 
this?  I don't remember approving any such change to doc encoder, but maybe I 
missed it in a review?

This will need to be undone, so I'm wondering what previous bugfix is dependent 
on it.
Whiteboard: fixinhand
cvsblame shows it was badami@netscape.com, sr'd by jst.  That patch has to be
backed out.  patch to do that is here.
Nominating for EDITORBASE and nsbeta1 since this is 
a) fixinhand
b) a backout of a recent patch, so there is no regression risk (other than bug
original patch was for: 62188, which is not nearly as serious as this bug).
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: nsbeta1
Whiteboard: fixinhand → EDITORBASE; fixinhand; need r=,sr=,a=
Keywords: nsbeta1 → nsbeta1+
Whiteboard: EDITORBASE; fixinhand; need r=,sr=,a= → EDITORBASE+; fixinhand; need r=,sr=,a=[adt2]
Target Milestone: mozilla1.0.1 → mozilla1.0
It was from Tanu's bug 62188, and I reviewed an earlier version of the patch for
that bug which also had this problem.  Whoops, sorry!

I'm hoping that if we back out just this part, that the rest of the fix for bug
62188 will continue to work in the case where multiple list items are chosen
(i.e. if you select part of a list item, the list item and list don't go into
the copy context, but if you select a complete list item, they do).  Yes?  No? 
We should probably go ahead and back it out, but let's make sure we know whether
we're regressing bug 62188 or not.
Well, It just looks like a typo -- nsHTMLAtoms::ul probably was meant to be used
instead of nsHTMLAtoms::li

Making that change (below) fixes this bug and does not regress bug 62188.

Index: base/src/nsDocumentEncoder.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/base/src/nsDocumentEncoder.cpp,v
retrieving revision 1.66
diff -u -1 -r1.66 nsDocumentEncoder.cpp
--- base/src/nsDocumentEncoder.cpp      23 Mar 2002 23:54:44 -0000      1.66
+++ base/src/nsDocumentEncoder.cpp      29 Mar 2002 00:47:09 -0000
@@ -1307,3 +1307,3 @@
       tag.get() == nsHTMLAtoms::ol       ||
-      tag.get() == nsHTMLAtoms::li) {
+      tag.get() == nsHTMLAtoms::ul) {
     return PR_TRUE;
Attached patch Patch to fix this... (obsolete) — — Splinter Review 
The bug is a regression caused my patch for bug#62188. This patch is to fix it
in the same way as discussed above. It carries few more changes to resolve the
inconsistency in indentation of elements in OL and UL. More specifically,
earlier if I copied "b" from <OL>abc</OL>, on pasting it to some Text Editor, I
was getting(with indentation)
      b     
but for <UL>abc</UL> I was getting(without indentation)
b
With the patch both are pasted as:
b

Instead of opening a new bug, here I'm including a fix for that too.
Neither ol or ul (or li) can remain in IncludeInContext().  That will force even
one letter selections in the middle of a list item to emit a list into the
clipboard data.  And now that will be a list without a list item, with text
directly in the list: not even legal html.

Any patch that goes in for this has to have no list elements in
InludeInContext().  If that regresses 62188, that's ok - this bug is worse.  You
get the indention of "b" in Tanu's example above because of the text directly in
a list (with no list item) - that's how browsers have historically rendered that
illagal html, but it doen't mean we should generate it.

I need r's and sr's for the original patch I attached here.  

kin, can u sr this? 
*** Bug 134257 has been marked as a duplicate of this bug. ***
*** Bug 134475 has been marked as a duplicate of this bug. ***
>Neither ol or ul (or li) can remain in IncludeInContext().

Without these tags, my fix for 62188 has no meaning. 

>That will force even one letter selections in the middle of a list item to emit 
>a list into the clipboard data. And now that will be a list without a list 
>item, with text directly in the list: not even legal html.

This is true. So we need to decide if we want to avoid generating an illegal 
html as mentioned above or loose a feature that 62188 adds. 
>So we need to decide if we want to avoid generating an illegal 
>html as mentioned above or loose a feature that 62188 adds.

Tanu, that decision is easy.  Mozilla is all about standards compliance.  We
need to output valid HTML, much as I would hate to see 62188 backed out.

>>Neither ol or ul (or li) can remain in IncludeInContext().
>Without these tags, my fix for 62188 has no meaning.

jfrancis, I didn't investigate further when I posted comment 9, but I do see
now.  However we should probably be looking at a full backout of 62188 and not
just this one file, since Tanu says the other changes are void without the
nsDocumentEncoder changes.

I'd hate to see 62188 regress, but in order to fix this bug, it is the right
thing unless an alternate solution to 62188 is found quickly.
I couldn't imagine it would be so hard to implement the copy of list markers as
in Bug 62188

I've tried to examine the behaviour of Internet Explorer 6 and Opera 6 for
curiosity and in fact they both only copy the texts ignoring <li> tags.

Anyway I think there could be a workaround. For example, neither ol or ul (or
li) would be included in IncludeInContext() if only a single line has been
selected and there's at least another character before than the first character
selected, so that ol or ul would always be included as it happens now with an
exception.
In response to Tanu's comment #14, this is not really a choice between just 
illegal html or a fix for 62188.  If we take Tanu's patch, you will get bad 
results whenever you copy from inside a list item and do a plaintext paste.  It 
would paste on a new line, indented.  

Plaintext serialization basically needs some extra info on the clipboard in order 
to solve 62188.  When we needed extra info for html paste inside composer, we 
added some extra data flavors on the clipboard to hold that info (see 
EncodeToStringWithContext()).  

Ironically, this is just the kind of problem that XIF would have been good at 
solving.

akkana, can i get an r= here?  this is fairly serious bug with simple fix.  kin
or jst, can i get sr?
Comment on attachment 76729 [details] [diff] [review]
Patch to fix this...

Reluctant r=akkana because it is a regression.	Sigh.

We really need to find a solution to this problem, though!  The inability to
list any of these items in the copy context is causing a large number of other
bugs like the one Tanu was fixing -- we need to find a way to include that
information somehow.
Attachment #76729 - Flags: review+
*** Bug 135235 has been marked as a duplicate of this bug. ***
Comment on attachment 76663 [details] [diff] [review]
undoing earlier patch of nsDocumentEncoder.cpp

This was actually the patch I was reviewing.
Attachment #76663 - Flags: review+
Comment on attachment 76729 [details] [diff] [review]
Patch to fix this...

Mistakenly marked this patch as reviewed ... un-marking (though I'd love to be
able to keep this one).
Attachment #76729 - Flags: review+
Comment on attachment 76663 [details] [diff] [review]
undoing earlier patch of nsDocumentEncoder.cpp

sr=kin@netscape.com

Please reopen bug 62188
 if you check this in.
Attachment #76663 - Flags: superreview+
Whiteboard: EDITORBASE+; fixinhand; need r=,sr=,a=[adt2] → EDITORBASE+; fixinhand; need a=[adt2]
See also bug 78676, "selection inside a single <li> shouldn't include bullet". 
The recent change makes bug 78676 trigger more often, but that doesn't mean it's
at fault.
Jfrancis, I think in nsDocumentEncoder, if we can do something like the parent 
tag is included only if the child node is completely selected, we would get the 
right thing. This is something that Akkana has always suggested for all the tags 
in general. It would resolve quite some table related bugs and many more. As I 
recall, it could take quite some time, which we all agree and we may not like to 
do that for 1.0. 

So can we make a compromise at this stage? I think we can, because even though 
we know that something like <ol>abc</ol> is incorrect yet we know that most of 
the browsers are able to resolve this and do the right thing with this and such 
situations are much lesser than the ones where we are doing copy/paste with the 
list items.
Keywords: adt1.0.0, approval
Comment on attachment 76663 [details] [diff] [review]
undoing earlier patch of nsDocumentEncoder.cpp

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76663 - Flags: approval+
adt1.0.0+ (on ADT's behalf) approval for checkin into 1.0.
Keywords: adt1.0.0 → adt1.0.0+
Tanu, the idea of having certain tags force certain ancestors into the copy
stream is probably a good one, but we can not do that for moz1.0.  In the
meatime, I can't leave any list structure in IncludeInContext().  Even if you
excluded li, and only did ol/ul, it would still break paste.
Comment on attachment 76729 [details] [diff] [review]
Patch to fix this...

we cant do this
Attachment #76729 - Attachment is obsolete: true
fix landed on trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Still pastes as # when copying from middle of a list: bug 137482
This landed before branch cut.  Hence: fixed1.0.0
Keywords: approval → fixed1.0.0
using trunk build from 2003040708 on win2K, this works correctly. I created a
test case with the initial list descriptions, viewed the test case in the
browser, selected partial list item content and pasted into notepad. Only the
text was pasted, not the list item marker.
Status: RESOLVED → VERIFIED
I confirm that the list item marker is not copied (OS X 2003040708).

However, I do note that if you select a word manually, an extra space gets copied.

eg on the originator's web page, select 'Item' by placing the cursor to the
right of the 'm', click and hold the left mouse button, and moving left until
the cursor has just moved to the left of the 'I', such that the whole word
'Item' is selected, so as you can be sure that no 'space' before the 'I' has
been selected.

Copy and paste that into another window, and it will have a space before the 'I'.

I also notice that when you cut and paste the entire list, an extra space is
inserted between the '#' character and the 'I' of 'Item'.

Also wondering why the '#'s aren't numbers, since I was copying an ordered list
(from the first list on http://www.elfqrin.com/Sample.html#Lists)

Different bug(s) perhaps?

Max.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: