-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mod_Wrapper multiple instances #19136
Conversation
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. |
Just turn that Id to a class to solve the multiple instances problem (it’s a b/c break by the way). |
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) |
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:
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 |
@@ -1,7 +1,7 @@ | |||
function iFrameHeight() | |||
{ | |||
var height = 0; | |||
var iframe = document.getElementById('blockrandom'); | |||
var iframe = document.querySelectorAll('*[id^="blockrandom"]'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove spaces at the end of the line.
IMO the call to iframeHeight should be made with the ID as a parameter so that the JS can find the absolutely correct iframe. |
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. |
it is relevant 😉 |
@brianteeman 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
|
okay, I see, |
@brianteeman check brianteeman#67 |
Thanks
@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 ✅ successfully on d81bea8. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19136. |
I have tested this item ✅ successfully on d81bea8 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19136. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19136. |
Thanks |
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 )
Summary of Changes
Testing Instructions
Expected result
Actual result
Documentation Changes Required