940568 - Starting to navigate away from a document with an editor disables the editor, even if the navigation is later aborted

Status: NEW

(Core :: DOM: Editor, defect, P5)

Description

[Jos de Jong]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131028112810

Steps to reproduce:

Try this jsfiddle: http://jsfiddle.net/6ehqY/2/

Or create an HTML page containing: 
- A link, having a data link and a download attribute like so:
    <a href="data:text/plain,helloworld" download="hello.txt">click here</a>
- An editable div:
    <div contenteditable="true">type text here</div>
- Try to copy/paste text in the editable div. This works.
- Now click the link and after that, try again to copy/paste text in the editable div. This is not possible anymore.





Actual results:

The copy/paste functionality of the editable div disappears after clicking the link.


Expected results:

The editable div should still have copy/paste functionality after clicking the link.

Comment 1

[(no longer active)]

Can you please test this on Firefox Nightly?  I can't reproduce it there.

Comment 2

[Alice0775 White]

I cannot "Paste" and "Cut" in latest holly build.  
But "Copy" seems OK.

http://hg.mozilla.org/projects/holly/rev/eab4eb855bf8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131119093712

Comment 3

[(no longer active)]

(In reply to comment #2)
> I cannot "Paste" and "Cut" in latest holly build.  
> But "Copy" seems OK.

What's holly?

Comment 4

[Alice0775 White]

holly = m-c - Australis

now available in http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/

Comment 5

[Jos de Jong]

@ehsan: I just tested on Firefox Nighly, same results. 

You can see it very easily from the context menu: right click in an editable div normally gives a context menu containing a section Cut/Copy/Paste/Delete, but after clicking the link with data and download attribute, this section is suddenly gone.

Comment 6

[(no longer active)]

You're right, I can reproduce now.  Tom, any idea what's going on here?

Comment 7

[Tom S [:evilpie]]

Sorry, I don't know. I would be interesting to test some normal URL that triggers a download, but doesn't have the attribute. If that doesn't trigger this, then that would be a good starting point. In all honesty I don't really understand how document loading works enough to say what the problem is.

Comment 8

[Boris Zbarsky [:bzbarsky]]

This has nothing to do with @download per se, as expected.  Here's a minimal testcase:

 <a href="data:application/octet-stream,">Click me</a>
 <div contenteditable="true"
      style="border: 1px solid gray; width: 300px; height:200px;">I am editable,</div>

Ehsan, does editor listen for beforeunload events by any chance?  Because it's not just copy/paste that's broken; the div stops being editable entirely.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yes, it effectively does:

#0  nsDocShell::DetachEditorFromWindow (this=0x122179c00) at nsDocShell.cpp:7383
#1  0x0000000106b61f2c in non-virtual thunk to nsDocShell::DetachEditorFromWindow() () at nsDocShellEditorData.cpp:221
#2  0x00000001053eede7 in nsEditingSession::StartDocumentLoad (this=0x14c0cdc10, aWebProgress=0x122179c28, aIsToBeMadeEditable=true) at nsEditingSession.cpp:905
#3  0x00000001053ee987 in nsEditingSession::OnStateChange (this=0x14c0cdc10, aWebProgress=0x122179c28, aRequest=0x115bf42a0, aStateFlags=983041, aStatus=NS_OK) at nsEditingSession.cpp:693
#4  0x00000001053ef645 in non-virtual thunk to nsEditingSession::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, tag_nsresult) () at nsEditingSession.cpp:1096
#5  0x00000001047485c8 in nsDocLoader::DoFireOnStateChange (this=0x122179c00, aProgress=0x122179c28, aRequest=0x115bf42a0, aStateFlags=@0x7fff5fbfa89c, aStatus=NS_OK) at nsDocLoader.cpp:1331
#6  0x0000000104747bac in nsDocLoader::FireOnStateChange (this=0x122179c00, aProgress=0x122179c28, aRequest=0x115bf42a0, aStateFlags=983041, aStatus=NS_OK) at nsDocLoader.cpp:1268
#7  0x00000001047473b8 in nsDocLoader::doStartDocumentLoad (this=0x122179c00) at nsDocLoader.cpp:782
#8  0x00000001047471bb in nsDocLoader::OnStartRequest (this=0x122179c00, request=0x115bf42a0, aCtxt=0x0) at nsDocLoader.cpp:485
#9  0x0000000104747507 in non-virtual thunk to nsDocLoader::OnStartRequest(nsIRequest*, nsISupports*) () at nsDocLoader.cpp:808
#10 0x0000000103dd195c in nsLoadGroup::AddRequest (this=0x12495ed50, request=0x115bf42a0, ctxt=0x0) at nsLoadGroup.cpp:566
#11 0x0000000103d9365f in nsBaseChannel::AsyncOpen (this=0x115bf4250, listener=0x14acc9c40, ctxt=0x0) at nsBaseChannel.cpp:614
#12 0x0000000103d936d7 in non-virtual thunk to nsBaseChannel::AsyncOpen(nsIStreamListener*, nsISupports*) () at nsBaseChannel.cpp:619
#13 0x00000001047524de in nsURILoader::OpenURI (this=0x10f4889e0, channel=0x115bf42a0, aFlags=1, aWindowContext=0x122179c30) at nsURILoader.cpp:785


Just because a load _started_ doesn't mean the document is going away!

Comment 10

[(no longer active)]

Wow!  The editor code *still* manages to surprise me.  :-)

What is the right notification to wait for here?

Comment 11

[Boris Zbarsky [:bzbarsky]]

That's a good question...  We already detach in the docshell code on pagehide (nsDocShell::FirePageHideNotification).  So maybe we can just remove this codepath altogether?  Why was it added?

Comment 12

[(no longer active)]

(In reply to comment #11)
> That's a good question...  We already detach in the docshell code on pagehide
> (nsDocShell::FirePageHideNotification).  So maybe we can just remove this
> codepath altogether?  Why was it added?

That predates me.  I think Peter wrote this stuff, and I wouldn't dare just removing it...  It's unfortunately difficult to reason about why this is needed. :(

Comment 13

[Boris Zbarsky [:bzbarsky]]

Well, what does the blame say?

Comment 14

[(no longer active)]

Not much, typical of the editor code: https://github.com/mozilla/mozilla-central/commit/28e47e6e

Comment 15

[Boris Zbarsky [:bzbarsky]]

Ah, that old.  :(  That's unfortunate....

Comment 16

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.