Last Comment Bug 599322 - Base href ignored for drag/drop or copy/paste in designMode
: Base href ignored for drag/drop or copy/paste in designMode
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: P3 normal (vote)
: mozilla2.0b7
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
http://tinymce.moxiecode.com/gecko/ba...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-24 06:25 PDT by Spocke
Modified: 2010-09-28 15:25 PDT (History)
5 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+
.11+
.11-fixed
.14+
.14-fixed


Attachments
Patch (v1) (4.43 KB, patch)
2010-09-24 12:03 PDT, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
christian: approval1.9.2.11+
christian: approval1.9.1.14+
Details | Diff | Review

Description Spocke 2010-09-24 06:25:04 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 ( .NET CLR 3.5.30729; .NET4.0E)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 ( .NET CLR 3.5.30729; .NET4.0E)

The <base href=".."> gets ignored when you perform a drag/drop or copy/paste inside a designMode area. This affects existing products like TinyMCE and this didn't happen before the 3.6.9 minor release. I would guess that the mfsa2010-62 has something to do with this.

Reproducible: Always

Steps to Reproduce:
1. Open the attached URL.
2. Drag/drop or copy/paste the images inside the iframe.
3. Notice that the image becomes broken.
Actual Results:  
The images are broken after a drag/drop since the URLs are relative to the current page not the base path.

Expected Results:  
The images should have the URL intact or be relative to the right location.

I guess this one messed things up:
http://www.mozilla.org/security/announce/2010/mfsa2010-62.html

It might be better to not filter the contents in drag/drop or copy/paste on the same domain if possible. Since elements can contain other things you want to retain such as custom attributes.
Comment 1 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-24 08:09:44 PDT
This doesn't have anything to do with the fix to MSFA2010-62.  The same problem is reproducible in 3.6.8 using the test case in this bug as well.
Comment 2 Spocke 2010-09-24 08:26:09 PDT
Ahh we had a fix in TinyMCE for this issue where we use a custom attribute called _mce_src that holds the original url. We restore the correct url by using that attribute on drop. The MSFA2010-62 strips all attributes except valid ones so that custom attribute gets removed and then our fix doesn't work any more. I guess should file a new bug report regarding that but this issue is the original bug that prompted us to do the workaround.
Comment 3 christian 2010-09-24 10:31:22 PDT
So you should be able to use a HTML5 data attribute instead of a custom one (bug 598105) to workaround, correct?
Comment 4 Spocke 2010-09-24 10:41:20 PDT
Yes, but existing applications that uses TinyMCE would break such as Wordpress and there is a lot of users out there that would get upset. We use the _mce_ prefix similar to the _moz_ prefix in TinyMCE and it would be ideal if they where retained. But the attribute filtering issue is being discussed at a separate bug report: https://bugzilla.mozilla.org/show_bug.cgi?id=596300

So please remove the reference to MSFA2010-62 from this bug if possible since it doesn't have any relevance to this one. This one is only about the base href being ignored when dropping or pasting contents.
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-24 11:55:56 PDT
Spocke, I have a very safe patch (it's a one-liner code wise) which solves this issue, and I think we can take that patch on the branch.  But until the next dot-release, you can use Christian's suggestion.

Christian, may I recommend that this bug blocks the next dot release for both branches?
Comment 6 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-24 12:03:07 PDT
Created attachment 478351 [details] [diff] [review]
Patch (v1)

GetDocBaseURI returns the base URI if one is available, or the document URI if it's not.  With this, the base URI if present ends up overriding the document URI, which is what we want.
Comment 7 christian 2010-09-27 10:31:31 PDT
Marking as blocking. Ehsan, do you think this will come in by tomorrow night, or do you think it should be deferred until the next one?
Comment 8 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-27 10:54:05 PDT
http://hg.mozilla.org/mozilla-central/rev/b1a8e09a13f4
Comment 9 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-27 10:54:22 PDT
(In reply to comment #7)
> Marking as blocking. Ehsan, do you think this will come in by tomorrow night,
> or do you think it should be deferred until the next one?

It will hopefully come in by tonight.  :-)
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-27 15:17:54 PDT
Bustage fix:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8d42a507bb40
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/372481ff0103
Comment 12 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-27 19:44:06 PDT
Test fix followup on branches: SimpleTest.waitForFocus is not available on branches, so I just replaced it with a setTimeout.

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ee78fb6afe02
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/09e1a8d09661
Comment 13 Wolfgang Rosenauer [:wolfiR] 2010-09-27 23:42:42 PDT
apparently SimpleTest.waitForClipboard is also not available? See oranges.
Comment 14 Mark Banner (:standard8) 2010-09-28 01:03:02 PDT
I've backed this out from 1.9.2 due to persisting test failures:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/d59afde40273

It looks like 1.9.1 could do with a similar backout/fix...
Comment 15 Mark Banner (:standard8) 2010-09-28 01:17:27 PDT
Sorry, this bug is still fixed on trunk, so shouldn't have reopened it - the status1.9.2 flag should be enough to track this.
Comment 16 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-28 09:13:47 PDT
(In reply to comment #13)
> apparently SimpleTest.waitForClipboard is also not available? See oranges.

Sigh.  Without SimpleTest.waitForClipboard, this is not possible to test reliably.  I guess I'll have to steal waitForClipboard's code from trunk and add it to the test.
Comment 17 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-28 11:15:32 PDT
Landed with test fixes:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4e4a45a5045d
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/8d0accc2738f
Comment 18 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-28 11:18:08 PDT
Test fix ported to 1.9.1:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/944aa1640267
Comment 19 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-28 12:16:30 PDT
Yet another follow-up to work around the fact that querySelector is not available on 1.9.1.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a95320254fbb
Comment 20 :Ehsan Akhgari (busy, don't ask for review please) 2010-09-28 15:25:46 PDT
Argh, pushed one final 1.9.1 fix which makes the test pass for me locally:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5d57c5cc2033

Note You need to log in before you can comment on or make changes to this bug.