User tests: Successful: Unsuccessful:
Pull Request for Issue #17425
This is a PARTIAL fix. It addresses the issue of the unique id for each occurrence of a wrapper module but as noted in the original issue the JS is broken as well
It looks to me that we can change the selector to the following
var iframe = document.querySelectorAll('*[id^="blockrandom"]');
but there are still issues and I am NOT a js guru (pinging @dgt41 )
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End |
Just turn that Id to a class to solve the multiple instances problem (itβs a b/c break by the way).
About the chrome error: itβs expected unless you properly set the allowed scripts in your site. There is nothing to fix here
Sorry I don't understand you?
Either your approach here or a class driven solution is b/c break! Think that someone got an override then either solution will break the overrides...
turning the id to a class would be an a11y fail as every iframe has to have a unique id which was the whole point of the pr
@brianteeman can you please add also the js part (the code in the desc is fine)
Labels |
Added:
?
|
Category | Modules Front End | ⇒ | JavaScript Modules Front End |
js added
You can still have an ID on the iframe. Just add a class too and target the class in the JS as opposed to the ID
After applying this fix I get:
iframe-height.js:5 Uncaught TypeError: Cannot use 'in' operator to search for 'contentDocument' in null
at iFrameHeight (iframe-height.js:5)
at HTMLIFrameElement.onload ((index):809)
Page has two iframes - one within com_wrapper, and one inserted into a module by a different plugin.
See #19337 for environment.
iirc the code i wrote here only addresses multiple instances of the wrapper module. your bug sounds unrelated
IMO the call to iframeHeight should be made with the ID as a parameter so that the JS can find the absolutely correct iframe.
iirc the code i wrote here only addresses multiple instances of the wrapper module. your bug sounds unrelated
No - the error message I reported after trying this fix only appears with this fix and is unrelated to the cross-domain issue I have reported separately.
iirc the code i wrote here only addresses multiple instances of the wrapper module. your bug sounds unrelated
it is relevant
after your changes var iframe
contain multiple iframe
element instead of 1, but whole code expect 1 element.
The code need more changes to support multiple instances.
@brianteeman I am not tested, but this should work: ignore it
function iFrameHeight()
{
var iframe, doc,
height = 0,
isAll = !!document.all,
iframes = document.querySelectorAll('iframe[id^="blockrandom"]');
for (var i = 0, l = iframes.length; i < l; i++) {
iframe = iframes[i];
doc = 'contentDocument' in iframe ? iframe.contentDocument : iframe.contentWindow.document;
height = parseInt(doc.body.scrollHeight);
if (!isAll)
{
iframe.style.height = height + 60 + 'px';
}
else
{
document.all[iframe.id].style.height = height + 20 + 'px';
}
}
}
The problem with this code is that it will run on every iframe every time any iframe finishes loading.
As stated previously IMO the code should call iFrameHeight with the id as a parameter iframeHeight(id)
and the code should then read:
function iFrameHeight(id)
{
var height = 0;
var iframe = document.getElementById('blockrandom-' + id);
var doc = 'contentDocument' in iframe ? iframe.contentDocument : iframe.contentWindow.document;
if (!document.all)
{
height = doc.body.scrollHeight;
iframe.style.height = parseInt(height) + 60 + 'px';
}
else if (document.all)
{
height = doc.body.scrollHeight;
document.all.blockrandom.style.height = parseInt(height) + 20 + 'px';
}
}
okay, I see,
then it can be even easier
@brianteeman check brianteeman#67
@Sophist-UK you asked for changes, changes have been made so please test
I have tested this - still getting the x-domain console error which is to be expected, but aside from that it seems to work.
@Sophist-UK Please mark your test result here: https://issues.joomla.org/tracker/joomla-cms/19136
I have tested this item
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-01-16 22:40:06 |
Closed_By | ⇒ | mbabker | |
Labels |
Added:
?
|
Thanks
I have tested this itemβ
successfully on 0f908f0
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19136.