Closed
Bug 647682
Opened 14 years ago
Closed 14 years ago
Cutting and pasting of links fails in composer edit pages
Categories
(SeaMonkey :: Composer, defect)
Tracking
(seamonkey2.3 fixed, seamonkey2.4 fixed, seamonkey2.5 fixed)
RESOLVED
FIXED
seamonkey2.3
People
(Reporter: mns, Assigned: neil)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ehsan.akhgari
:
review+
neil
:
feedback+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; nl-nl) AppleWebKit/533.20.25 (KHTML, like Gecko) Version/5.0.4 Safari/533.20.27
Build Identifier: 2.0.12 and 2.0.13
I have reported this to the SeaMonkey Council, but they couldn't reproduce it. Me and a friend of mine on another Mac OSX 10.6.7 mac mini snowleopard, her in the Netherlands have the same problem with the Dutch version of the program. Cutting and pasting links results in the clipboard not keeping the link. So when copying texts we have to redo all the links. Therefore we have reverted to version 2.0.11 which works fine on our machine and in our language. Can anybody reproduce and or fix this problem?
Reproducible: Always
Steps to Reproduce:
1.Mac OS 10.6.7 minimac (old version)
2.Open html page with composer
3.Trying to cut and paste a link within the same page and between pages, results in failure of clipboard to keep the link.
Actual Results:
no links in the text pasted
Expected Results:
keep the link of the the text pasted
version 10.0.11 works fine, from then on we have this problem
Comment 1•14 years ago
|
||
Sure, I can confirm this bug. Same thing happened with BlueGriffon.
Assignee | ||
Comment 2•14 years ago
|
||
Thanks to Daniel I can reproduce this now. What I hadn't realised was that the target of the link needs to be a local file, or an anchor in a local file (including a relative anchor within the document itself...)
I'm guessing that this is another regression from bug 520189. It might be dependent on bug 606117.
Blocks: CVE-2010-2769
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Mac OS X → All
Hardware: Other → All
Version: unspecified → SeaMonkey 2.0 Branch
Assignee | ||
Comment 3•14 years ago
|
||
As a workaround you can copy and paste the source of the link, either via the HTML source view (which is easier to copy from) or the Insert HTML dialog (which is easier to paste source with).
Assignee | ||
Comment 4•14 years ago
|
||
And the reason this fails is because pasting doesn't provide a document to InsertFromTransferable so that it's always considered to be unsafe. (What do you do when you want to paste a link copied from an external application?)
Comment 5•14 years ago
|
||
(In reply to comment #4)
> And the reason this fails is because pasting doesn't provide a document to
> InsertFromTransferable so that it's always considered to be unsafe. (What do
> you do when you want to paste a link copied from an external application?)
yeah... If that bit of code is understandable in the browser, it makes little
sense in the editor where js and plugins are disabled by default.
We should check the docshell to see if it's an editorshell and in that case
assume the link is always safe. What do you think?
Comment 6•14 years ago
|
||
(In reply to comment #5)
> (In reply to comment #4)
> > And the reason this fails is because pasting doesn't provide a document to
> > InsertFromTransferable so that it's always considered to be unsafe. (What do
> > you do when you want to paste a link copied from an external application?)
>
> yeah... If that bit of code is understandable in the browser, it makes little
> sense in the editor where js and plugins are disabled by default.
> We should check the docshell to see if it's an editorshell and in that case
> assume the link is always safe. What do you think?
Yeah, we already have a similar treatment for images <http://mxr.mozilla.org/comm-central/source/mozilla/content/base/src/nsContentUtils.cpp#2440>, so I guess I could live with that. Are you going to write the patch, or should I?
Assignee | ||
Comment 7•14 years ago
|
||
Attachment #526380 -
Flags: feedback?(ehsan)
Comment 8•14 years ago
|
||
Comment on attachment 526380 [details] [diff] [review]
Possible patch
This look good to me. Having some comment there describing what the purpose is would make it even better.
You should probably ask Boris to review it.
Attachment #526380 -
Flags: feedback?(ehsan) → feedback+
Assignee | ||
Updated•14 years ago
|
Attachment #526380 -
Flags: feedback?(bzbarsky)
Comment 9•14 years ago
|
||
Comment on attachment 526380 [details] [diff] [review]
Possible patch
Yeah, I guess this works. God, we need to rationalize our docshell interfaces.
Attachment #526380 -
Flags: feedback?(bzbarsky) → feedback+
Assignee | ||
Comment 10•14 years ago
|
||
Comment on attachment 526380 [details] [diff] [review]
Possible patch
ehsan, I know you asked for extra comments; what about extra error-checking? Or indeed is there anything else you'd like me to do to the patch?
Comment 11•14 years ago
|
||
Comment on attachment 526380 [details] [diff] [review]
Possible patch
Review of attachment 526380 [details] [diff] [review]:
Since you asked me to be stricter... ;-)
::: editor/libeditor/html/nsHTMLDataTransfer.cpp
@@ +1317,5 @@
+ nsCOMPtr<nsIDocShellTreeItem> root;
+ dsti->GetRootTreeItem(getter_AddRefs(root));
+ nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(root));
+ PRUint32 appType;
+ docShell->GetAppType(&appType);
This will leave appType uninitialized if GetAppType fails, I think.
Assignee | ||
Comment 12•14 years ago
|
||
With extra error checking as requested.
Hopefully r=ehsan f=bz suffices.
Assignee: nobody → neil
Attachment #526380 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #528432 -
Flags: review?(ehsan)
Attachment #528432 -
Flags: feedback+
Comment 13•14 years ago
|
||
Comment on attachment 528432 [details] [diff] [review]
Proposed patch
Yes, they do suffice. :-)
Attachment #528432 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 14•14 years ago
|
||
Pushed changeset 7893933cf5d0 to mozilla-central.
Actually I pushed it yesterday but I forgot to update the bug.
As it's a Gecko 2 regression, do we want this on any branches?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 15•14 years ago
|
||
(In reply to comment #14)
> Pushed changeset 7893933cf5d0 to mozilla-central.
>
> Actually I pushed it yesterday but I forgot to update the bug.
>
> As it's a Gecko 2 regression, do we want this on any branches?
I don't know if we're planning another dot-release on 2.0, but since this patch doesn't affect Firefox, I don't think anybody would mind. Not sure if you need to go through with the approval2.0 flag or not though.
Comment 19•13 years ago
|
||
The fix for this propagated to current mozilla-beta, so it's fixed for 2.3. Don't know about earlier versions.
status-seamonkey2.3:
--- → fixed
status-seamonkey2.4:
--- → fixed
status-seamonkey2.5:
--- → fixed
Target Milestone: --- → seamonkey2.3
You need to log in
before you can comment on or make changes to this bug.
Description
•