Opened 11 years ago
Closed 5 years ago
#9622 closed enhancement (fixed)
ContentPane: pre-processing callback for downloaded content
Reported by: | Foam Head | Owned by: | bill |
---|---|---|---|
Priority: | high | Milestone: | 1.11 |
Component: | Dijit | Version: | 1.3.2 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description (last modified by )
I'm working with a ContentPane that downloads its content via an href, but the content needs to be pre-processed before it is displayed. Despite having onLoad and onDownloadEnd callbacks, both of these are called *after* the content is displayed.
It looks like having onLoad fire after the content is displayed is identical in function to onDownloadEnd. I'm wondering if onLoad should be fired before the content is displayed. If onLoad and onDownloadEnd need to remain as is, then a new callback (onPreLoad? onDownloadPreprocess? preprocessDownload? preprocessContent?) should be added.
Note: My experience with this has been with a ContentPane, but the same pre-processing callback should be available in all classes that support downloading href content.
Attachments (1)
Change History (14)
comment:1 Changed 11 years ago by
Milestone: | tbd → 1.5 |
---|
comment:2 Changed 11 years ago by
Changed 11 years ago by
Attachment: | ContentPane.patch added |
---|
comment:3 Changed 11 years ago by
Milestone: | 1.5 → 1.6 |
---|
comment:4 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Milestone: | 1.6 → 1.7 |
Summary: | RFE: pre-processing callback for downloaded content → ContentPane: pre-processing callback for downloaded content |
comment:6 Changed 9 years ago by
Milestone: | 1.8 → future |
---|
comment:7 Changed 5 years ago by
Milestone: | future → 1.11 |
---|---|
Owner: | set to dylan |
Status: | new → assigned |
This oneliner looks like it should have been accepted and fixed 6 years ago. I'll review and land for 1.11, unless bill wants to handle it.
comment:8 Changed 5 years ago by
It's unfortunate that Dojo 1.3 broke backwards compatibility with Dojo 1.1, but now this patch breaks backwards compatibility with Dojo 1.3 - 1.10, because existing code may assume the content is available (in this.containerNode
) when onDownloadEnd()
is called. So I don't think I want to change the current behavior of onDownloadEnd()
.
Having a new callback like preprocessContent(origContent)
that returns the new content seems reasonable. You could call it from _setContent()
. That means though that it would get called for inlined content too (from .set("content", ...)
etc.), and sometimes the argument could be a DocumentFragment
instead of a String
.
On a related note, onLoad()
is a little different than onDownloadEnd()
because onLoad()
is also called for .set("content", ...)
.
comment:9 Changed 5 years ago by
Owner: | changed from dylan to bill |
---|
Bill, I've added https://github.com/dojo/dijit/pull/106
Please let me know if this is what you had in mind. I didn't add a test, as it's just a stub/placeholder, so doesn't really add any new functionality unless the user overrides it.
If you think this is better handled not as an event handler, but just as a callback, let me know and I'll change it.
comment:10 Changed 5 years ago by
A few things:
- It should have a test case, even though it's just a callback, in the same way that we already test
onDownloadStart()
andonDownloadEnd()
. Specifically,Contentpane-remote.html
tests those functions in the "onDownloadStart|End" test, so you should add some lines to that test to check thatpreprocessContent()
is working too.
- As I wrote in my previous comment, I meant for the function to return the modified content (or by default, the original content). After all, that's what this ticket is about: pre-processing the content before we set innerHTML. I guess that means that it's "just a callback" and should be named
preprocessContent()
rather thanonPreprocessContent()
.
comment:11 Changed 5 years ago by
ok, that makes more sense. I'll have an updated PR in the next day or two. Thanks!
comment:12 Changed 5 years ago by
bill, I've created an updated PR. The tests may be too much in that each test is preprocessing the content, so we can scale that back if needed.
I vote for this change. In Dojo 1.1.0 the sequence of caling was as follows:
and for me it was correct.
Now (DOJO 1.3.2 and up):
so, onDownloadEnd is useless now, becouse is called just after onLoad.
I attached simple patch, whitch fixes this problem, is it correct approach?