-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Transition of Joomla's Wrapper from HTML4 to HTML5 #19965
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
Conversation
Changed "scroll" and "scrolling" to "overflow" and "frameborder" and "border" (required by HTML5 standard).
|
@BaleshSrle What do you think about changing it like this:
|
|
Isn't this a B/C break with the changing of param names/values? |
|
Then why did you inserted HTML5 Shiv in Joomla? It's job is to make the websites with html5 supportable and presentable to older web browsers (for example Internet Explorer 8 or even Internet Explorer 6). There is no need for HTML4 code, because the htm5shiv is bridging the gap between the older web browsers (pre-HTML5) an new HTML5 code structure. For more info, you can see (and even search) on some web links: For those who want to see does their Joomla web-site supports HTML5 coding standard, you can find tools @ W3C Markup Validator Service & Html Validator for Firefox and Chrome. P.S.:Inform me if you want my contribution to Joomla 4 iframe element, if you want to modernise it. If not, then I promise to you all that I will modify Joomla's iframe element after every Joomla update. |
Yes. As implemented, this PR can't be accepted because it will cause existing sites using the wrapper component/module to stop using their existing configurations and switch to the default HTML5 configurations after an update. To be accepted in 3.x what @ReLater suggested needs to be applied. |
Why not just changing your provided code once like suggested and making all users happy including yourself? ;-) Please leave a comment here if you don't want to change it. Maybe(!) I'll find the time to take it over. |
|
@BaleshSrle I'm not disagreeing with you. I'm simply saying it's a B/C break as you're changing parameter names which will effect those with template overrides for those views. |
|
@C-Lodder & @mbabker I have 17 iframe (or wrapper) elements on my 2 web-sites all together, and it took me all together less than 6 hours (maybe even less) to change all settings in every wrapper element from start to finish. Maybe you all are lazy and don't want to work on your web sites (that means to improve and maintain them). I don't know about you, but I like to work on my web-sites. About the template override issue, let's force the template designers to start creating templates by using HTML5 coding standard. BTW, before I created this PR, I sent an email to [email protected] informing them about this and I sent them compressed modified xml and php files, and Luca Marzo sent me an e-mail and he informed me that I need to create a PR on Joomla's GitHub. @ReLater I started this, and I will finish this. If you wish, you can join & assist me (because I don't understand PHP, but I understand HTML). But have in mind that this needs to be lite, because some people, who are using self-hosting servers, don't have enough storage space on those servers. |
|
It's not about being lazy, it's not about how much effort it might take a site owner to apply the changes. We have a backward compatibility promise, ONLY in a major version (i.e. 4.0) can breaking changes be accepted (preferably with a migration path if applicable). For this change proposal to land in a 3.x release, it must be made backward compatible. That means updating this PR in a way where existing HTML4 based configurations will keep working and users who elect to update their sites are free to do so with the new parameters. Otherwise, this PR can only be applied to 4.0 if there is going to be no effort in making it backward compatible. |
|
@mbabker I think that you are B/C this to Joomla 2.5.28. :-D But if you like, I can create a PR for 4.0 , but after taking a glimpse at that code, I can say it is about 50% (or even more) similar to 3.x |
| height="<?php echo $this->escape($this->params->get('height')); ?>" | ||
| scrolling="<?php echo $this->escape($this->params->get('scrolling')); ?>" | ||
| frameborder="<?php echo $this->escape($this->params->get('frameborder', 1)); ?>" | ||
| style="width <?php echo $this->escape($this->params->get('width')); ?>; height: <?php echo $this->escape($this->params->get('height')); ?>; overflow: <?php echo $this->escape($this->params->get('overflow')); ?>; border: <?php echo $this->escape($this->params->get('border', 1)); ?>;" |
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.
Missing colon : after width
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.
Hawk eyes ;-)
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.
Hawk eyes ;-)
Error is changed...
| height="<?php echo $this->escape($this->params->get('height')); ?>" | ||
| scrolling="<?php echo $this->escape($this->params->get('scrolling')); ?>" | ||
| frameborder="<?php echo $this->escape($this->params->get('frameborder', 1)); ?>" | ||
| style="width <?php echo $this->escape($this->params->get('width')); ?>; height: <?php echo $this->escape($this->params->get('height')); ?>; overflow: <?php echo $this->escape($this->params->get('overflow')); ?>; border: <?php echo $this->escape($this->params->get('border', 1)); ?>;" |
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.
Hawk eyes ;-)
|
@BaleshSrle Nothing to do with being lazy at all. It's simply obiding by semver rules. Your changes will be fine against the |
|
@BaleshSrle |
|
Set to "closed" on behalf of @Quy by The JTracker Application at issues.joomla.org/joomla-cms/19965 |
|
Closing in favor for #19985. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/19965. |
|
reopened |
|
A strange find with my pr #19985 but also relevant for yours if you plan to commit it for Joomla 4 Most browsers don't support |
|
See also pr #19991 (mod_wrapper, Joomla 3) |
|
@ReLater You didn't read the link about overflow, because there is a table which displays the minimum version of web browser that supports CSS overflow property. By the way, there is an HTML Shiv in Joomla. |
You still don't understand. If a user chooses setting That's the only criterion for your and my pr (!!!!) |
|
It does hide it... If it doesn't, then change the dimensions of iframe element (I mean on height and/or width of iframe) and leave overflow: hidden. |
|
Still the same answer: In Joomla 3 we have to guarantee that a user controls the behavior in backend like before. I've tested that with current Joomla 3 and HTML5 doesn't work like expected without adding HTML4 attribute That's a known browsers issue, BTW. I've closed my prs a month ago after some inverstigations because I don't like this attribute mix. But you can find their some "hints" to keep a Wrapper pr B\C for Joomla 3. So, please feel free to provide a testable HTML5 version for Wrapper with the same common settings in backend for Joomla 3 or Joomla 4 or both. Makes no sense if we discuss theoretical things here ;-) |
|
Look, I don't know what is the issue, but I suggest you open
in Mozilla-base (ie Firefox, Pale Moon or Sea Monkey) or Chromium-base (ie Google Chrome, Vivaldi or Opera) web browser and find the issue in my web-sites source code, because I can't find any. |
|
Due to the nature of the link you posted I have had to remove it. Please dont post a similar link again. |
Summary of Changes
Made a transition in Joomla's IFRAME element from HTML4 to HTML5 standard
Testing Instructions
Added wrapper modules on index.php and in menus (and/or submenus).
Using "px" or "%" in "height" field (required by HTML5 standard).
Expected result
Working 100 %
Actual result
Works like a charm, as seen @ view-source:https://sflogistika.dobojcaffe.com/v2/ & view-source:https://sflogistika.dobojcaffe.com/v2/index.php/video/srce-za-saobracajni
Documentation Changes Required
Need to modify 6 files to work without any problems or hiccups:
modules/mod_wrapper/mod_wrapper.xml
modules/mod_wrapper/mod_wrapper.php
modules/mod_wrapper/helper.php
modules/mod_wrapper/tmpl/default.php
components/com_wrapper/views/wrapper/tmpl/default.xml
components/com_wrapper/views/wrapper/tmpl/default.php