193716 - unlink doesn't work correctly (cmd_removeLinks)

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[kaldari]

The unlink command doesn't work and throws the following JavaScript error:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMNSHTMLDocument.execCommand]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
http://www.angelblade.com/midas/editor.html :: tbclick :: line 194"  data: no]

Comment 1

[Kathleen :Brade]

The url in the original comment is not a valid url (at least for me).  Demo2
seems to have an attempt to demonstrate the problem but there seems to be JS
problems which need to be fixed before someone can see the ExecCommand failure.

Comment 2

[kaldari]

Sorry, didn't quite have a testcase ready. Try this...
http://www.angelblade.com/midas/tester.html

Comment 3

[kaldari]

BTW, in case it's not clear, the unlink button is the one with the breaking
chain link in the top toolbar.

Comment 4

[Kathleen :Brade]

Attachment: register remove links command

Comment 5

[Mike Kaply [:mkaply]]

Comment on attachment 114706 [details] [diff] [review]
register remove links command

We'd REALLY like this for 1.3. We are finalizing the midas API and this one is
hanging out.

Thanks

Easy one line change.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 114706 [details] [diff] [review]
register remove links command

a=asa (on behalf of drivers) for checkin to 1.3.

Comment 7

[Kathleen :Brade]

fix checked in

Comment 8

[kaldari]

The behavior of the unlink command is now as follows:
The first time you call it, it puts <href></href> tags around the link but
doesn't remove the anchor tags.
If you call it again, it removes the href and anchor tags.

Here's how I'm calling it:
execCommand(unlink, false, null)

Comment 9

[kaldari]

Perhaps the registration should be:
NS_REGISTER_STYLE_COMMAND(nsStyleUpdatingCommand, "cmd_removeLinks", "a");
rather than:
NS_REGISTER_STYLE_COMMAND(nsStyleUpdatingCommand, "cmd_removeLinks", "href");

Just a guess really :)

Comment 10

[Kathleen :Brade]

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

Comment 11

[Kathleen :Brade]

Attachment: patch so commands handle href/name specially like editor does

Comment 12

[Kathleen :Brade]

Attachment: more efficient version of previous patch

Comment 13

[kinmoz]

Comment on attachment 116101 [details] [diff] [review]
more efficient version of previous patch

sr=[e-mail]

I know you changed the href/name comment. I would probably move it under the
|if| href and name block, but it's up to you.

Does doTagRemoval need to be initialized? Or does the GetBooleanValue() call
guarantee initialization?

Comment 14

[Kathleen :Brade]

fix checked in

Comment 15

[Kathleen :Brade]

Comment on attachment 116101 [details] [diff] [review]
more efficient version of previous patch

seeking approval to land this fix for 1.3 (if it's not too late); I'm seeing
dupes of bugs filed on it already...

it affects composer as well as midas

Comment 16

[kaldari]

This patch may be causing a problem with the more important createlink command.
I'll post a better explaination in a second...

Comment 17

[kaldari]

When I was testing out the latest Nightly in Windows, it seems that when you
create a link now with the createlink command, it doesn't show up as a link
until you switch to source mode and back or apply some kind of formatting to the
link.

Steps to reproduce:
1. Go to http://www.angelblade.com/midas/tester.html with the latest Nightly
2. Select some text
3. Click the link button

What should happen: The text turns blue and appears underlined
What actually happens: Nothing (at least visibly)

If you swicth to source mode you can see that an anchor tag was actually
created, and if you switch back it will appear blue and underlined. Also if you
apply some kind of formatting like bold or italic to the link it will suddenly
turn blue and get an underline. It seems to work fine in the 1.3b release.

Comment 18

[kaldari]

BTW, I'm not sure this patch is actually to blame for the above problem, but I
wanted to have someone check on it before the patch goes into 1.3. Also, I
forgot to mention that this patch does actually fix the unlink command for me.

Comment 19

[Kathleen :Brade]

I'm sure there is a different bug floating around for the links not being
underlined... that bug has been around for a few days and isn't related to this
bug at all.  (Sorry I don't have the bug # handy nor time to query for it.)

Comment 20

[kaldari]

The link bug appears to be present in the 03-03 trunk build as well, so I'm
guessing this patch isn't to blame after all.

Comment 21

[kaldari]

I've filed a new bug on the createlink issue: bug 195948

I don't think this patch has anything to do with it. I recommend approving this
patch for 1.3 as it does fix unlink.

Comment 22

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

Comment on attachment 116101 [details] [diff] [review]
more efficient version of previous patch

Does the code modified by this patch affect anything other than Midas?	(If so,
what?)

Comment 23

[Kathleen :Brade]

Composer and Mail compose are broken on 1.3 and also fixed by this patch.

Comment 24

[Christopher Blizzard (:blizzard)]

Comment on attachment 116101 [details] [diff] [review]
more efficient version of previous patch

a=blizzard for the 1.3 branch

Comment 25

[Mike Kaply [:mkaply]]

Fix checked into branch.

Comment 26

[Kathleen :Brade]

mkaply checked in the patch on the 1.3 branch earlier today

marking bug as verified for trunk based on comments from Ryan and others; if
someone could comment here that 1.3 is now also fixed, that'd be great.

Comment 27

[kaldari]

I have verified that unlink is fixed in 1.3 now. Thanks!