Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#8125 closed defect (fixed)

ContentPane attr('href') cancels itself!

Reported by: Matt Sgarlata Owned by: bill
Priority: high Milestone: 1.2.3
Component: Dijit Version: 1.2.2
Keywords: Cc: development-staff@…
Blocked By: Blocking:

Description

attr('href') incorrectly calls the cancel() method when the request is still in the middle of being processed. The practical consequence of this is that the _xhrDfd member variable is cleared before anything can be done with it. This makes it difficult to work with requests initiated by ContentPane? and makes it impossible to inspect the response headers of those requests.

This is a regression in dojo 1.2.2 that was not a problem in 1.2 beta. This was broken when the changes for bug 7801 were checked in. [Note the version # of this bug is 1.2.1; it looks like 1.2.2 needs to be added to Trac]

http://trac.dojotoolkit.org/ticket/7801

In case someone would like to argue that _xhrDfd shouldn't be around when onLoad and onDownloadEnd are called, it's clear the original author intended _xhrDfd to be around when onLoad and onDownloadEnd are called because it is manually deleted in _downloadExternalContent after onLoad and onDownloadEnd have had a chance to run.

Currently the rough order of operations is something like this

  • attr('href')
  • dojo.xhrGet
  • attr('content')
    • cancel
    • onLoad
  • onDownloadEnd

The problem is that attr('content') should definitely be calling cancel, but attr('href') should not. So the _downloadExternalContent method needs to be modified so that it does *not* call attr('content'). Perhaps the current contents of the _setContentAttr method could be moved to some new helper method (let's call it _setContentHelper) that could be accessed by both attr('content') and attr('href'). So attr('content') would just call cancel() and then _setContentHelper and attr('href') would just call _setContentHelper in place of attr('content') and thus avoid the cancel call.

I attached a test case that demonstrates that cancel is called before onLoad and onDownloadEnd have a chance to run. It also demonstrates that the _xhrDfd member variable is undefined in onLoad and onDownloadEnd (which is not correct, as mentioned earlier). When you execute the test case, you'll notice that the cancel method is actually called 3 times. The first 2 calls are OK, because they just make sure any prior attr('href') calls are cancelled, which definitely does need to happen. It's the 3rd one that's a problem because it is called before onLoad or onDownloadEnd.

Attachments (1)

dojoBugOnDownloadEndTest.html (1.2 KB) - added by Matt Sgarlata 11 years ago.
Test case

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by Matt Sgarlata

Test case

comment:1 Changed 11 years ago by Matt Sgarlata

Sorry, I fail at wiki formatting. Here is another try at explaining the order of operations:

  • attr('href')
  • dojo.xhrGet
  • attr('content')
    • cancel
    • onLoad
  • onDownloadEnd

comment:2 Changed 11 years ago by bill

Milestone: tbd1.2.3
Owner: set to bill
Status: newassigned
Version: 1.2.11.2.2

Yes, you are right, thanks for pointing this out.

I've just checked in fixes to this on trunk ([15794]) and on the 1.2 branch ([15795]). See if that fixes the problem for you and if you see any other issues.

I have noticed some other failures though for the dojox.ContentPane test so I'm looking at those now.

comment:3 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

OK, fixed a typo in [15797] and those test failures (which were really test file problems) in [15798].

I think this is working now but let me know if it isn't.

comment:4 Changed 11 years ago by Matt Sgarlata

Thanks, Bill! I was hoping to check this out today, but the latest nightly build online is for 11/19/2008. As soon as there's one for the 20th I will test this out.

comment:5 Changed 11 years ago by Matt Sgarlata

Actually I finally got myself hooked up to SVN and pulled down revision 15826 and this is working great for me :) Thanks again!

comment:6 Changed 11 years ago by bill

Good, glad it's working. Keep letting us know about isues (especially detailed reports like this).

Note: See TracTickets for help on using tickets.