Open Bug 940568 Opened 8 years ago Updated 4 months ago

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

Categories

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

20 Branch
x86_64
All
defect

Tracking

()

People

(Reporter: wjosdejong, Unassigned)

References

()

Details

(Keywords: regression)

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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Editor
Ever confirmed: true
OS: Linux → All
Product: Firefox → Core
Blocks: 676619
Keywords: regression
Version: 25 Branch → 20 Branch
Can you please test this on Firefox Nightly?  I can't reproduce it there.
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
(In reply to comment #2)
> I cannot "Paste" and "Cut" in latest holly build.  
> But "Copy" seems OK.

What's holly?
@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.
You're right, I can reproduce now.  Tom, any idea what's going on here?
Flags: needinfo?(evilpies)
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.
Flags: needinfo?(evilpies)
Component: Editor → Document Navigation
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.
Flags: needinfo?(ehsan)
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!
Component: Document Navigation → Editor
Flags: needinfo?(ehsan)
Summary: Copy/paste in editable div disabled after clicking a link with download attribute → Starting to navigate away from a document with an editor disables the editor, even if the navigation is later aborted
Wow!  The editor code *still* manages to surprise me.  :-)

What is the right notification to wait for here?
Flags: needinfo?(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?
Flags: needinfo?(bzbarsky)
(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. :(
Well, what does the blame say?
Not much, typical of the editor code: https://github.com/mozilla/mozilla-central/commit/28e47e6e
Ah, that old.  :(  That's unfortunate....

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.

Severity: normal → S4
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.