[MAGNOLIA-3110] javascript document.write causing head elements to render in DOM body Created: 08/Mar/10 Updated: 23/Jan/13 Resolved: 09/Mar/10 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | admininterface |
| Affects Version/s: | 4.2.3 |
| Fix Version/s: | 4.3 |
| Type: | Bug | Priority: | Major |
| Reporter: | Nickolaus Wing | Assignee: | Magnolia International |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | 3h | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | 3h | ||
| Environment: |
All browsers, XHTML 1.0 Strict |
||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Patch included: |
Yes
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
In the admin-interface module, inline.js contains 3 document.write statements that create divs for later use by javascript. If we use <cms:links/> in the <head> of the page, this javascript will be included in the head. Then, the browser will execute those document.writes during the loading of the <head> element, writing out divs that belong in the <body>. This causes all known browsers to begin the body element immediately in their internal representation. Thus all tags following <cms:links/> will appear in the <body>, according to the DOM. This has always been annoyingly messy when using something like firebug to examine a document, but it also caused serious problems when including Scriptaculous, which attempts to find itself in the <head> in order to include more files, which obviously it can't do if it's been moved to the <body>. It can be solved with the attached patch, which moves the creation of these divs to the window.onload event (in a crossplatform, unobtrusive fashion). The file to be patched is m-m-admininterface/.../admin-js/inline.js |
| Comments |
| Comment by Magnolia International [ 08/Mar/10 ] |
|
I think there's already a similar function to execute functions onload, so maybe the one in your patch will be redudant. Thanks though! I also seem to remember a similar issue.. that I'll try to find and link to this one asap. |
| Comment by Magnolia International [ 08/Mar/10 ] |
|
As of 4.2.3 and Safari 4.0.4, I still get this:
which should hopefully be resolved with your patch. Thanks! |
| Comment by Magnolia International [ 08/Mar/10 ] |
|
Looks like I was wrong, can't find an existing method to add event listeners - i guess i mixed things up with STK/jQuery. |
| Comment by Nickolaus Wing [ 08/Mar/10 ] |
|
Yeah, I looked about for one for a little while and didn't find anything. It's definitely a handy function to have around, but I put it right there to try to keep the patch tightly contained. It should probably be placed adjacent to other similar utility functions in generic.js or something. |
| Comment by Nickolaus Wing [ 08/Mar/10 ] |
|
I submitted another admin-interface javascript patch in http://jira.magnolia-cms.com/browse/MAGNOLIA-1182. I wasn't sure whether to update that (very old) ticket or create a new one. Thought I'd draw your attention to it since the fix is right there in the same general area as this one. |
| Comment by Magnolia International [ 09/Mar/10 ] |
|
Finally found the method i thought we had: see magnolia-module-admininterface/src/main/resources/mgnl-resources/js-classes/mgnl/util/DHTMLUtil.js#addOnLoad - i assume your patch could work with this too, right ? |
| Comment by Magnolia International [ 09/Mar/10 ] |
|
applied patch, using our less generic addOnLoad method - not too keen on adding js functionality as we're headed towards major changes with 5.0. |