? Success

User tests: Successful: Unsuccessful:

avatar laoneo
laoneo
11 Aug 2016

Summary of Changes

Auto height is broken for the wrapper menu item.

Testing Instructions

  • Create a wrapper menu item with a local link like demo/1-article.
  • In the advanced options of the wrapper menu item set Auto Height to yes.
  • Open the wrapper menu item on the front. #### Expected result

The iframe is resized that not scrollbars are visible.

Actual result

The iframe doesn't get resized.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Aug 2016
Category JavaScript
avatar laoneo laoneo - open - 11 Aug 2016
avatar laoneo laoneo - change - 11 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 11 Aug 2016
Labels Added: ?
avatar laoneo laoneo - change - 11 Aug 2016
The description was changed
avatar laoneo laoneo - edited - 11 Aug 2016
avatar brianteeman
brianteeman - comment - 11 Aug 2016

This needs to be tested fully on all browsers, especially IE, as the iframe stuff works differently across the browsers

avatar laoneo
laoneo - comment - 11 Aug 2016

I'v developed it with Chrome on Ubuntu Linux.

avatar brianteeman
brianteeman - comment - 11 Aug 2016

Hence the need for extensive testing - this is based on experience ;)

On 11 August 2016 at 12:32, Allon Moritz notifications@github.com wrote:

I'v developed it with Chrome on Ubuntu Linux.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#11554 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8f62Pj5Y2puSsWfH3MSDokGNMSB5ks5qewhQgaJpZM4Jh8Dh
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar C-Lodder
C-Lodder - comment - 11 Aug 2016

Not working on IE10 or below

avatar C-Lodder
C-Lodder - comment - 11 Aug 2016

Update the code to this:

function iFrameHeight()
{
    var height = 0;
    var iframe = document.getElementById('blockrandom');
    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';
    }
}

Works on all browsers. There is a slight scroll on IE8 still, so whether or not you want to extend the above code to fix full or leave it is up to you

avatar laoneo
laoneo - comment - 11 Aug 2016

Updated the code as suggested by @C-Lodder. Thanks for that!

avatar C-Lodder C-Lodder - test_item - 11 Aug 2016 - Tested successfully
avatar C-Lodder
C-Lodder - comment - 11 Aug 2016

I have tested this item successfully on 4c213a3


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11554.

avatar jeckodevelopment
jeckodevelopment - comment - 11 Aug 2016

Please test on different browsers:

  • Microsoft Edge on Windows 10
  • IE 11 on Windows
  • Mozilla Firefox (latest) on Windows
  • Mozilla Firefox (latest) on MacOS
  • Mozilla Firefox (latest) on GNU/Linux
  • Google Chrome on Windows
  • Google Chrome on MacOS
  • Opera (latest) on Windows
  • Chromium on Linux
  • Safari on Windows
  • Safari on MacOS
  • any other welcome
avatar C-Lodder
C-Lodder - comment - 11 Aug 2016
  • IE8 - nearly successful (not 100% height but nearly)
  • IE9 - successful
  • IE10 - successful
  • IE11 - successful
  • Edge : Win 10 - successful
  • Firefox : Windows - successful
  • Chrome : Windows - successful
  • Opera : Windows - successful
  • Yandex : Windows - successful

I don't have a Mac or Linux but all browsers (apart from slight issue on IE8) work perfectly fine.

Please note in future we must test on Yandex as Joomla states they support it.

avatar jeckodevelopment
jeckodevelopment - comment - 11 Aug 2016

AFAIK Yandex Browser is based on Chromium, so compatibility should be mostly the same.
Btw, thank you @C-Lodder for introducing me this browser :)

avatar brianteeman
brianteeman - comment - 11 Aug 2016

And don't forget safari

avatar C-Lodder
C-Lodder - comment - 11 Aug 2016

@jeckodevelopment - same goes for Opera ;)
@brianteeman - Only on Mac though, not Windows

avatar jeckodevelopment
jeckodevelopment - comment - 11 Aug 2016

@brianteeman thank you!

avatar laoneo
laoneo - comment - 12 Aug 2016

As I'm not allowed to do testing, so just a remark. It works on Ubuntu with Firefox and Chrome. But please somebody else should confirm.

avatar C-Lodder
C-Lodder - comment - 12 Aug 2016

Macs at work so have tested on them:

  • Firefox : Mac - successful
  • Chrome : Mac - successful
  • Safari : Mac - successful
avatar brianteeman
brianteeman - comment - 12 Aug 2016

:( unable to test with patchtester

Could not connect to GitHub: No commit found for the ref iframe-auto-height


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11554.

avatar brianteeman brianteeman - test_item - 12 Aug 2016 - Not tested
avatar brianteeman
brianteeman - comment - 12 Aug 2016

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11554.

avatar brianteeman
brianteeman - comment - 12 Aug 2016

ignore that message it was a bug in patchtester 3

avatar brianteeman brianteeman - test_item - 15 Nov 2016 - Tested successfully
avatar brianteeman
brianteeman - comment - 15 Nov 2016

I have tested this item successfully on 4c213a3

- [ ] chrome osx

avatar C-Lodder
C-Lodder - comment - 15 Nov 2016

RTC?

avatar zero-24 zero-24 - change - 15 Nov 2016
The description was changed
Milestone Added:
Status Pending Ready to Commit
Labels Added: ?
avatar zero-24 zero-24 - change - 15 Nov 2016
Milestone Added:
avatar zero-24 zero-24 - change - 15 Nov 2016
Labels
avatar zero-24 zero-24 - edited - 15 Nov 2016
avatar brianteeman
brianteeman - comment - 15 Nov 2016

@C-Lodder patience ;)

avatar dgt41
dgt41 - comment - 15 Nov 2016

@laoneo since you are here can you put a deprecation notice about the blockrandom id, so in J4 we can introduce a real random id or class?

avatar rdeutz rdeutz - change - 15 Nov 2016
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2016-11-15 21:31:50
Closed_By rdeutz
avatar rdeutz rdeutz - close - 15 Nov 2016
avatar rdeutz rdeutz - merge - 15 Nov 2016
avatar rdeutz rdeutz - reference | e6b85d1 - 15 Nov 16
avatar rdeutz rdeutz - merge - 15 Nov 2016
avatar rdeutz rdeutz - close - 15 Nov 2016
avatar laoneo
laoneo - comment - 16 Nov 2016

@dgt41 I guess the bot called rdeutz faster, the merge is done already ?

avatar laoneo laoneo - head_ref_deleted - 16 Nov 2016
avatar rdeutz
rdeutz - comment - 16 Nov 2016

Thanks for calling me a bot, great motivation to spend my evening to check and merge PR

avatar dgt41
dgt41 - comment - 16 Nov 2016

@rdeutz I think Alon meant that the bot ping you, not you are a bot named rdeutz ?

avatar laoneo
laoneo - comment - 16 Nov 2016

Yes I was refering the joomla bot who is adding the RTC labelt. Fixed my comment, was too early in the morning, should not try then to make jokes.

avatar brianteeman
brianteeman - comment - 16 Nov 2016

It is not a BOT adding the label it is a human!!

There are several of us that manually review a PR before it is marked as RTC and between us we check many times a day.

Please don't scream for something to be RTC within minutes of a test. In many case we don't even get the time to get the email for a successful test before we get the shouting.

Although a PR nominally needs just two testers we do also apply some discretion on that to prevent two people in the same office or even same family from saying they have tested stuff.
Marking something as RTC is usually not just a case of flipping a switch

If it doesnt get marked RTC within 24 hours then please comment - there may be a valid reason but it may have been missed but I dont think we have missed more than 1 or 2 of the thousand plus PRs

avatar zero-24
zero-24 - comment - 16 Nov 2016

@dgt41 @laoneo i'm not sure what is missing now. But there should be no problem just send the missing stuff as new PR?

avatar laoneo
laoneo - comment - 17 Nov 2016

@brianteeman don't know what you are talking about, but I always tough that this is some hook script and not a real human
image

The whole thing got merged before I could add the code change suggested by @dgt41. I was only talking about this and not more. Don't wanted to offend anybody. Hope all clear now and we can move on.

avatar zero-24
zero-24 - comment - 17 Nov 2016

The bot do this on our command. Some of the people that have the permission tonset RTC does not have the permission to change labes on github. So we have implemented a bot that can do that too.

avatar xvinaumx
xvinaumx - comment - 23 Sep 2017

Sorry to revive this but I feel that I should share this:

I was encountering a problem with Joomla iframe wrapper. The auto height was working in IE (Version 11.608.15063.0) but not in Chrome (Version 61.0.3163.100 - it was passing just the "60px" part). After googling a lot and finding this page, I've found the js in Joomla directory and tried to find a solution. What worked to me was changing "height = doc.body.scrollHeight" to "height = document.body.scrollHeight". Now its working in IE and Chrome.

Regards


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11554.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 24 Sep 2017

@xvinaumx can you please open a new Issue as writng on closed Issues get low Attention.

Add a Comment

Login with GitHub to post a comment