Bug 7683 - TinyMCE: Implement execCommand(UnLink)
Summary: TinyMCE: Implement execCommand(UnLink)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Justin Garcia
URL:
Keywords:
Depends on:
Blocks: 6627
  Show dependency treegraph
 
Reported: 2006-03-09 12:06 PST by Justin Garcia
Modified: 2006-03-25 03:30 PST (History)
0 users

See Also:


Attachments
patch (35.75 KB, patch)
2006-03-15 20:41 PST, Justin Garcia
no flags Details | Formatted Diff | Diff
patch (43.80 KB, patch)
2006-03-16 13:47 PST, Justin Garcia
justin.garcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Garcia 2006-03-09 12:06:08 PST
Implement execCommand(UnLink)
Comment 1 Justin Garcia 2006-03-15 20:41:50 PST
Created attachment 7103 [details]
patch
Comment 2 Justin Garcia 2006-03-16 02:32:25 PST
I noticed that I misspelled would't:
+// and Unlink, would't push the
I think that selectionAroundNode is a misnomer.  The function returns a selection that selects the contents of the passed node.  selectionFromNodeContents is more accurate. 
Comment 3 Justin Garcia 2006-03-16 02:44:52 PST
Comment on attachment 7103 [details]
patch

Eek, I forgot to svn add Unlink.{h,cpp}!  Removing the review flag until I check in a new patch.  Here are some other problems I noticed:

This should be a FIXME
+    // This pushes down anchors even if they are fully selected.  This is 
+    // less efficient, but makes this code much less complicated.

I now use the new form of the ApplyStyleCommand constructor in 3 places:
+    EditCommandPtr cmd(new ApplyStyleCommand(document(), new CSSMutableStyleDeclarationImpl(), static_cast<ElementImpl*>(newAnchorElement.get())));
+    applyCommandToComposite(cmd);
I should put a convenience method in CompositeEditCommand.

I should follow the style guidelines even in JS code:
+    }
+    else {
+        execUnlinkCommand();
+    }
+}

All I really care about in the test case is the result of innerHTML, so I should probably make the test use dumpAsText.
Comment 4 Justin Garcia 2006-03-16 13:47:17 PST
Created attachment 7118 [details]
patch

Implemented Unlink using ApplyStyleCommand in a remove only mode
Added EditActions for CreateLink/Unlink
Added a new constructor for ApplyStyleCommand to be used by CreateLink, Unlink and pushPartiallySelectedAnchorElementsDown, and reverted the other two constructors to deal only with normal style application.
Added code to push partially selected anchor elements down.  It's used by CreateLink because anchors can't be nested. It's used by Unlink because it's necessary to break a partially selected anchor into fully selected pieces so that Unlink can remove them.
Comment 5 Justin Garcia 2006-03-24 19:15:47 PST
Comment on attachment 7118 [details]
patch

dave r+'ed this assuming I made a few changes he suggested.