97155 - Disable creating links when selection doesn't have text or an image

Status: NEW

(SeaMonkey :: Composer, defect)

Description

[TucsonTester2]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0b; Windows NT 5.1)
BuildID:    20010821

Creating a table then highlight in table with mouse and creating a link gives 
the text for the link as mixed.  Creating a table then pressing ctrl-A,  then 
creating a link gives no text for link name.

Reproducible: Always
Steps to Reproduce:
1. Open a new composer page
2. Click on table and create a teble of 2 X 2 then press ok
3. Press Ctrl -A  to select all
4. Right click on mouse and salect create link

Actual Results:  No text is in table and the link name is blank

Expected Results:  To see (Mixed Fields) as the name of the link

If you highlight the table even thoough there is no text then try to creat a 
link it will give you "Mixed Fields" as the name of the link.

Comment 1

[Kathleen :Brade]

-->cmanske

Comment 2

[Charles Manske]

We should probably just disable creating links if selection contains no text
or images, since the link isn't actually created in those cases.

Comment 3

[Syd Logan]

spam composer change

Comment 4

[Charles Manske]

Milestone load balancing.

Comment 5

[Charles Manske]

Attachment: Smarter enabling of "Insert Link" command

ComposerCommands.js: Check selection to enable creating a link.
editorOverlay.xul: Moved "cmd_link" into commandset that gets checked when
selection changes.
EditorContextMenu.js: Be sure command enabling is done when context popup is
created.

Comment 6

[Charles Manske]

Changing summary for a better description.

Comment 7

[Kathleen :Brade]

what is the performance hit with this change?

Comment 8

[Charles Manske]

I put a test "dump" statement in the isCommandEnabled() to show that this does not
impact on performance when user is typing or moving caret around with arrow keys.
Impact on window startup: In mail compse, none until user clicks in the content
area, then all commands that change with selection changed are updated.
In Composer, it is updated along with all the other style commands and does not
appear to impact on new window start time (subjective - no timing done.)

Comment 9

[Simon Fraser [no longer active]]

It seems that the only reason you need to get the selection as text is to test 
its length:

+        var selText = window.editorShell.GetContentsAs("text/plain", 
gOutputSelectionOnly);
+        return (selText && selText.length > 0);

Isn't there a more efficient way to get the length of the selected text?

Comment 10

[Simon Fraser [no longer active]]

What we really need is a 'CanInsertLink' API on the editor. Failing that, I think 
this workaround is OK. r=sfraser

Comment 11

[Charles Manske]

After reviews and discussion, we decided that getting the selection text is 
probably not a good strategy to test for link enabling. Technically, a link may
be created around just about any HTML content, and we will investigate why it
isn't always doing that now.

Comment 12

[Charles Manske]

From jfrancis on the dup bug 103957:
Charlie, I'm happy to help here but the first issue seems to be what is the link
dialog doing to determine if it needs to ask for link text?  I'm not really
familiar with the code there.  Hoping you can look into that and tell me what
editor call needs to behave differently for you, or perhaps what new
functionality you need.

Comment 13

[Charles Manske]

*** Bug 103957 has been marked as a duplicate of this bug. ***

Comment 14

[Charles Manske]

Joe: The link dialog always looks for "link text" when the selection is empty;
getting it by OutputToString(contentsAs, "text/plain", gOutputSelectionOnly);
Hence the suggested fix above that uses that to see if there's any text in the
selection. But we backed off from that since that can be an expensive call to
make just for enabling a command. 
So we're left with the problem of creating link(s)
correctly when selection spans block elements such as table cells, in which case
the <a> element(s) should be inserted under all block elements in the selection?
But even if we did this correctly, with <a> elements wrapping the <br> inside 
each cell of a table, there would not be any visible indication of the link until
a user typed something inside a table cell. Thus the original complaint, which
is technically invalid, imho,would still stand.
The only thing I think we should fix is to be sure inserting of <a> elements
throughout a selection obeys HTML parent/child rules.

Comment 15

[Joe Francis]

098

Comment 16

[Joe Francis]

pushing off 098 to 099

Comment 17

[Joe Francis]

the swami says: things that will not land in 099!

Comment 18

[Kevin McCluskey (gone)]

Moving bugs to Mozilla1.1 that are not EDITORBASE+.

Comment 19

[Kevin McCluskey (gone)]

Moz 1.1

Comment 20

[Joe Francis]

The trunk is the wave of the future!

Comment 21

[Joe Francis]

The days of having a half dozen milestones out in front of us to divide bugs 
between seem to be gone, though I dont know why.  Lumping everything together as 
far out as I can.  I'll pull back things that I am working on as I go.

Comment 22

[Joe Francis]

migrating milestones now that there are more available.  i'll pull these back as
I work on them