Moodle
  1. Moodle
  2. MDL-30769

URL resources to unknown types do not handle errors well

    Details

    • Rank:
      33700

      Description

      I would like to request change in mod/url. During addition of new url resource module tries to determine mime type served by URL. If determined type is not one from list of specially handled contents it uses general display method which is to embed the URL in this HTML tag:

      <div class="resourcecontent resourcegeneral">
      <object id="resourceobject" data="<url>" type="<mimetype>" width="800" height="600">
      $param
      $clicktoopen
      </object>
      </div>

      When you add a url resource to a course with location that does not exist (for example http://www3.open.ac.uk/courses/bin/p12.dll?C01S103) the error page will not be displayed to the user since most modern browsers refuse to load embedded page that returns HTTP code 404.

      We find that behavior not to our liking. We would prefer that error page to be displayed to the user. Would it be possible to add the setting where iframe usage could be forced for a specific URL and thus always displayed?

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Hi, Darko.

          I'm not sure what you mean there. It is possible to create links to different types of files. How each link is presented depends on its extension.

          Could you please explain what you think is lacking? Perhaps you could give us an example.

          Show
          Michael de Raadt added a comment - Hi, Darko. I'm not sure what you mean there. It is possible to create links to different types of files. How each link is presented depends on its extension. Could you please explain what you think is lacking? Perhaps you could give us an example.
          Hide
          Darko Miletic added a comment -

          Updated issue description. Let me know if this clears the doubts.

          Show
          Darko Miletic added a comment - Updated issue description. Let me know if this clears the doubts.
          Hide
          Michael de Raadt added a comment -

          Hi, Darko.

          Thanks for adding more to that. I have re-classified this as a bug, otherwise it won't get appropriate attention.

          Yes, the two solutions would be to either present the linked file in another way (like an iframe) or to detect there is a problem and present that to the user.

          Show
          Michael de Raadt added a comment - Hi, Darko. Thanks for adding more to that. I have re-classified this as a bug, otherwise it won't get appropriate attention. Yes, the two solutions would be to either present the linked file in another way (like an iframe) or to detect there is a problem and present that to the user.
          Hide
          Michael de Raadt added a comment -

          Carrying over to the next sprint.

          Show
          Michael de Raadt added a comment - Carrying over to the next sprint.
          Hide
          Ankit Agarwal added a comment -

          Hi Rosie,
          There is bunch of issues with current approach:-

          • fopen is disabled on most servers for remote fetching
          • We can use curl alternatively, but it will require a lot of processing, parsing https flags, get parameters etc
          • It is one additional request each time someone visits the url resource, which can be a huge performance hit, as it is making an external request.
          • fopen and curl are not always trust-able, since we are using server ip now to make the request instead of client ip, there can be specific ip based restriction on the external resource, which can lead to misleading results.

          We can implement a new setting in the drop down called "iframe", but that might lead to following issues:-

          • Accessibility concerns, Frames in general has bunch of accessibility issues associated with them and should be avoided.
          • Embed and Iframe options can be pretty confusing for common users.

          To conclude, I don't see any reasonable solution to this issue.
          Cheers
          Ankit

          Show
          Ankit Agarwal added a comment - Hi Rosie, There is bunch of issues with current approach:- fopen is disabled on most servers for remote fetching We can use curl alternatively, but it will require a lot of processing, parsing https flags, get parameters etc It is one additional request each time someone visits the url resource, which can be a huge performance hit, as it is making an external request. fopen and curl are not always trust-able, since we are using server ip now to make the request instead of client ip, there can be specific ip based restriction on the external resource, which can lead to misleading results. We can implement a new setting in the drop down called "iframe", but that might lead to following issues:- Accessibility concerns, Frames in general has bunch of accessibility issues associated with them and should be avoided. Embed and Iframe options can be pretty confusing for common users. To conclude, I don't see any reasonable solution to this issue. Cheers Ankit
          Hide
          Ankit Agarwal added a comment -

          Forgot to click the button
          Thanks

          Show
          Ankit Agarwal added a comment - Forgot to click the button Thanks
          Hide
          Rossiani Wijaya added a comment -

          Thank you Ankit for reviewing.

          Adding Sam for his input regarding this.

          Show
          Rossiani Wijaya added a comment - Thank you Ankit for reviewing. Adding Sam for his input regarding this.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I've just had a quick at this.
          My initial reaction is that fopen is not great, as Ankit notes fopen utilising protocols can be disabled.
          Also I don't like that this fopen call would be made for each call to resourcelib_embed_general, thats going to lead to it for every view of a resouce + url, and is going to be socketed death when rebuilding course modinfo (the call path leads back into modinfolib via url_get_coursemodule_info). That would lead to a fopen call being made for each resource in a course when creating the modinfo data.

          Looking back at what Darko is asking for, he is after a setting to force the use of an iframe.
          This sounds like a pretty smart idea to me, although I was not involved in the usability and accessibility work done in this area.
          But you could easilyi implement a new URL resource module setting to force the use of an iframe when embedding a page, and then within url_get_final_display_type check for such a setting and return a new const RESOURCELIB_DISPLAY_IFRAME. Then look for and deal with that explicitly in view.php. You could even move the iframe code from resourcelib_embed_general into a new function resourcelib_embed_iframe and make view.php and resourcelib_embed_general call that as required.
          On an interesting note the iframe element is included in the HTML working draft, notes as a nested browsing context, no doubt when we eventually get to HTML5 we will be moving away from the object tag back to the iframe for text/html mime types.

          Alternatively you could easily specify an alternative to display within the object when the objects content cannot be loaded.
          I had a quick test and you can even embed a secondary object tag in there.
          However a simple "URL not available" so something similar may suffice?
          What do you guys think of that? Would it meet your needs Darko?
          I'm know very little about objects, I've tested to make sure it si valid and sure enough it is fine, so perhaps that simple.
          Or the setting above.

          Either way there definitely would be a solution to this, and it is most certainly a bug as the output when the requested page doesn't exist is useless.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I've just had a quick at this. My initial reaction is that fopen is not great, as Ankit notes fopen utilising protocols can be disabled. Also I don't like that this fopen call would be made for each call to resourcelib_embed_general, thats going to lead to it for every view of a resouce + url, and is going to be socketed death when rebuilding course modinfo (the call path leads back into modinfolib via url_get_coursemodule_info). That would lead to a fopen call being made for each resource in a course when creating the modinfo data. Looking back at what Darko is asking for, he is after a setting to force the use of an iframe. This sounds like a pretty smart idea to me, although I was not involved in the usability and accessibility work done in this area. But you could easilyi implement a new URL resource module setting to force the use of an iframe when embedding a page, and then within url_get_final_display_type check for such a setting and return a new const RESOURCELIB_DISPLAY_IFRAME. Then look for and deal with that explicitly in view.php. You could even move the iframe code from resourcelib_embed_general into a new function resourcelib_embed_iframe and make view.php and resourcelib_embed_general call that as required. On an interesting note the iframe element is included in the HTML working draft, notes as a nested browsing context, no doubt when we eventually get to HTML5 we will be moving away from the object tag back to the iframe for text/html mime types. Alternatively you could easily specify an alternative to display within the object when the objects content cannot be loaded. I had a quick test and you can even embed a secondary object tag in there. However a simple "URL not available" so something similar may suffice? What do you guys think of that? Would it meet your needs Darko? I'm know very little about objects, I've tested to make sure it si valid and sure enough it is fine, so perhaps that simple. Or the setting above. Either way there definitely would be a solution to this, and it is most certainly a bug as the output when the requested page doesn't exist is useless. Many thanks Sam
          Hide
          Rossiani Wijaya added a comment - - edited

          Thanks Sam for your feedback.

          My +1 for adding new setting for iFrame, if it is really necessary to have.

          Show
          Rossiani Wijaya added a comment - - edited Thanks Sam for your feedback. My +1 for adding new setting for iFrame, if it is really necessary to have.
          Hide
          Ankit Agarwal added a comment -

          Hi Sam,
          I guess Iframes are not recommended considering accessibility. Still if everybody thinks it is a good idea, than we can go ahead with that.

          About the alternative text thing, are you suggesting to edit the "Click xxx link to open resource." to something more like " Cannot embed the url, Click xxxx link to open resource." ?

          Cheers

          Show
          Ankit Agarwal added a comment - Hi Sam, I guess Iframes are not recommended considering accessibility. Still if everybody thinks it is a good idea, than we can go ahead with that. About the alternative text thing, are you suggesting to edit the "Click xxx link to open resource." to something more like " Cannot embed the url, Click xxxx link to open resource." ? Cheers
          Hide
          Sam Hemelryk added a comment -

          No probs Rosie,
          Ankit if it becomes a setting the accessibility is less of a concern. We can add descriptive help that suggests the usability of an iframe isn't ideal but at the end of the day if people decide to enable it it would be a conscious decision by them to use the iframe, hopefully they will have considered their need to turn it on anyway.

          In regards to the alternative text, using a simple string like "Unable to load the requested URL" would be OK I think.
          Although ideally, and perhaps worth considering I'd suggest creating a url missing page within the URL resource that contained the text, and then using a nested object to display that as the alt (90% sure that works OK in all browsers we support).
          The advantage to that is that then we can make the missing page script log an event for the missing request (assuming here that the alt object doesn't get loaded unless needed, we'd need to test of course).

          Alternatively to both of those you could let the user decide as a setting of the URL module, although I would look to the above options first. This would work, but would be beating round the bush... why would you add a resource that could not be reached.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - No probs Rosie, Ankit if it becomes a setting the accessibility is less of a concern. We can add descriptive help that suggests the usability of an iframe isn't ideal but at the end of the day if people decide to enable it it would be a conscious decision by them to use the iframe, hopefully they will have considered their need to turn it on anyway. In regards to the alternative text, using a simple string like "Unable to load the requested URL" would be OK I think. Although ideally, and perhaps worth considering I'd suggest creating a url missing page within the URL resource that contained the text, and then using a nested object to display that as the alt (90% sure that works OK in all browsers we support). The advantage to that is that then we can make the missing page script log an event for the missing request (assuming here that the alt object doesn't get loaded unless needed, we'd need to test of course). Alternatively to both of those you could let the user decide as a setting of the URL module, although I would look to the above options first. This would work, but would be beating round the bush... why would you add a resource that could not be reached. Cheers Sam
          Hide
          Michael de Raadt added a comment -

          OK. So updating the text seems to be a positive step forward now.

          I think that if iframe is redeemed in future and becomes acceptable as part of HTML5 then we should use it, but until then the current solution is probably best.

          I'm not in favour of providing an option for the user to decide between an object or an iframe. It makes sense to us, but it would be meaningless to regular users.

          Show
          Michael de Raadt added a comment - OK. So updating the text seems to be a positive step forward now. I think that if iframe is redeemed in future and becomes acceptable as part of HTML5 then we should use it, but until then the current solution is probably best. I'm not in favour of providing an option for the user to decide between an object or an iframe. It makes sense to us, but it would be meaningless to regular users.
          Hide
          Rossiani Wijaya added a comment -

          Note: there is a glitch on displaying a loadable url in Master. The content page flicks when it gets display (eg: page is viewable then it disappears). I will create new issue to address the issue.

          Show
          Rossiani Wijaya added a comment - Note: there is a glitch on displaying a loadable url in Master. The content page flicks when it gets display (eg: page is viewable then it disappears). I will create new issue to address the issue.
          Show
          Rossiani Wijaya added a comment - Optional solution: https://github.com/rwijaya/moodle/compare/master...MDL-30769_withpage
          Hide
          Ankit Agarwal added a comment -

          First solution looks good.
          About second solution:-

          1. $id should be a required param in the page because you are using $cm->course later which wont be set if $id is not present
          2. there is no need of empty($id) check incase we make it a required param
          3. array('id'=>$cm->course) needs spaces
          4. array('id'=>$cm->instance) needs spaces
          5. you cannot use get_string('missingpage', 'url') as $action for add_to_log(), I am not sure what appropriate action it is. Normally we use view,edit,delete,update etc, but view doesnt help us notice that it is 404 page. It will be good to get some ideas on this.
          6. you will also need to define a new rule in log.php
          7. and increase version of url module to recognize that change.
          8. I am not sure if we need any header , footer and other things in this page. It will be good to get Sam's opinion on that.

          Thanks

          Show
          Ankit Agarwal added a comment - First solution looks good. About second solution:- $id should be a required param in the page because you are using $cm->course later which wont be set if $id is not present there is no need of empty($id) check incase we make it a required param array('id'=>$cm->course) needs spaces array('id'=>$cm->instance) needs spaces you cannot use get_string('missingpage', 'url') as $action for add_to_log(), I am not sure what appropriate action it is. Normally we use view,edit,delete,update etc, but view doesnt help us notice that it is 404 page. It will be good to get some ideas on this. you will also need to define a new rule in log.php and increase version of url module to recognize that change. I am not sure if we need any header , footer and other things in this page. It will be good to get Sam's opinion on that. Thanks
          Hide
          Ankit Agarwal added a comment -

          Hi Rosie,
          Both patchs look good to me now.

          Thanks for the changes.
          Feel free to submit for integration.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Rosie, Both patchs look good to me now. Thanks for the changes. Feel free to submit for integration. Thanks
          Hide
          Rossiani Wijaya added a comment -

          Thanks Ankit for reviewing.

          I'm submitting optional solution (embadded page) for integration.

          Also, assigning Sam as integrator

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Ankit for reviewing. I'm submitting optional solution (embadded page) for integration. Also, assigning Sam as integrator Submitting for integration review.
          Hide
          Sam Hemelryk added a comment -

          Hi Rossi,

          Sorry I am reopening this, its only over a very minor point but as it is Wednesday and it is security related I have no other option.

          I like the overall implementation, what I noted was the use of the msg param within the missing page script.
          It is being picked up as PARAM_RAW and then used directly in the page.
          That is a HUGE security risk as you could write anything you wanted into it have have it echo'd into the page.
          PARAM_RAW cleans data for use, it doesn't strip HTML and could leave people open to many different attacks there. You could construct quite the nasty link with that page presently.
          In thinking about it I don't see that need for that at the moment, perhaps sometime in the future someone will request it as a feature and we can add it to master, but for the time being I think it is better to leave it out entirely, it will make it easier to call this a bug fix as we're not introducing any functionality not required in order to fix the bug.

          Seeing as its back anyway could you please just confirm that you have tested this in all supported browsers.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rossi, Sorry I am reopening this, its only over a very minor point but as it is Wednesday and it is security related I have no other option. I like the overall implementation, what I noted was the use of the msg param within the missing page script. It is being picked up as PARAM_RAW and then used directly in the page. That is a HUGE security risk as you could write anything you wanted into it have have it echo'd into the page. PARAM_RAW cleans data for use, it doesn't strip HTML and could leave people open to many different attacks there. You could construct quite the nasty link with that page presently. In thinking about it I don't see that need for that at the moment, perhaps sometime in the future someone will request it as a feature and we can add it to master, but for the time being I think it is better to leave it out entirely, it will make it easier to call this a bug fix as we're not introducing any functionality not required in order to fix the bug. Seeing as its back anyway could you please just confirm that you have tested this in all supported browsers. Many thanks Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Rossiani Wijaya added a comment -

          Hi Sam,

          I made the changes according to your suggestion. However, removing $msg optional param, will limit the access of accessing the page manually (the url link will not display on the page).

          Therefore I created an alternative solution for this. Instead of using optional_param ($msg, '', PARAM_RAW), it would probably better to use optional_param($link, '', PARAM_URL) then print it out the link (just like the value for $clicktoopen). I know it seems like a redundancy for $clicktoopen, but at least the URL is available to be access manually.

          Let me know what do you think about the new patch.

          Thanks
          Rosie

          Updated patch for 2.2, 2.3 and 2.4.

          Show
          Rossiani Wijaya added a comment - Hi Sam, I made the changes according to your suggestion. However, removing $msg optional param, will limit the access of accessing the page manually (the url link will not display on the page). Therefore I created an alternative solution for this. Instead of using optional_param ($msg, '', PARAM_RAW), it would probably better to use optional_param($link, '', PARAM_URL) then print it out the link (just like the value for $clicktoopen). I know it seems like a redundancy for $clicktoopen, but at least the URL is available to be access manually. Let me know what do you think about the new patch. Thanks Rosie Updated patch for 2.2, 2.3 and 2.4.
          Hide
          Rossiani Wijaya added a comment -

          Side note:

          I tested the patch in IE (7, 8, and 9), instead of displaying the 'missing' page, it displays the browser 404 page. *see attachment

          In Firefox, Chrome and Opera for both Windows and Ubuntu, the page embedded the 'missing' page correctly. *see attachment

          Show
          Rossiani Wijaya added a comment - Side note: I tested the patch in IE (7, 8, and 9), instead of displaying the 'missing' page, it displays the browser 404 page. *see attachment In Firefox, Chrome and Opera for both Windows and Ubuntu, the page embedded the 'missing' page correctly. *see attachment
          Hide
          Rossiani Wijaya added a comment -

          Re-submit for integration.

          Show
          Rossiani Wijaya added a comment - Re-submit for integration.
          Hide
          Dan Poltawski added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.
          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.
          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P
          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Dan Poltawski added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Minor comment: global $DB; is not necessary in mod/url/missingpage.php

          I have some concerns about adding this log entry, because its only applicable when we embed a page, and well it might give the false impression that the resource module is checking that links are alive.

          Show
          Dan Poltawski added a comment - Minor comment: global $DB; is not necessary in mod/url/missingpage.php I have some concerns about adding this log entry, because its only applicable when we embed a page, and well it might give the false impression that the resource module is checking that links are alive.
          Hide
          Dan Poltawski added a comment -

          I'm refocusing on 2.4dev issues integrating so unassinging myself as integrator

          Show
          Dan Poltawski added a comment - I'm refocusing on 2.4dev issues integrating so unassinging myself as integrator
          Hide
          Sam Hemelryk added a comment -

          Hi Rosie,

          I've been playing around with this for a wee while now.
          In general I like the effect but I've noticed one gotcha to this.
          The missing page is ALWAYS loaded, regardless of whether the URL was loaded or not.
          It may be browser specific I don't know but I was using Firefox myself.
          The disadvantage to this is that all URL requests will load two pages, the missing page and the actual resource.
          Because the missing page is always loaded the log event is always added. Not a great outcome.
          Perhaps we could live with always loading it, but certainly the double log entries isn't a good outcome.
          I think we need to think about this one a bit more sorry.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rosie, I've been playing around with this for a wee while now. In general I like the effect but I've noticed one gotcha to this. The missing page is ALWAYS loaded, regardless of whether the URL was loaded or not. It may be browser specific I don't know but I was using Firefox myself. The disadvantage to this is that all URL requests will load two pages, the missing page and the actual resource. Because the missing page is always loaded the log event is always added. Not a great outcome. Perhaps we could live with always loading it, but certainly the double log entries isn't a good outcome. I think we need to think about this one a bit more sorry. Many thanks Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Rossiani Wijaya added a comment -

          Adding Petr as watcher.

          Hi Petr,

          could you provide some feedback on how should we fix this issue?

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Adding Petr as watcher. Hi Petr, could you provide some feedback on how should we fix this issue? Thanks Rosie
          Hide
          Petr Škoda added a comment -

          Hello, I do not understand this much, but why should you put object into object? Why not just put there a message indicating that either browser can not display the object or the object does not exist? Btw is there a way to find out if object was loaded or not? Maybe we could diagnose the loading from Javascript code...

          Show
          Petr Škoda added a comment - Hello, I do not understand this much, but why should you put object into object? Why not just put there a message indicating that either browser can not display the object or the object does not exist? Btw is there a way to find out if object was loaded or not? Maybe we could diagnose the loading from Javascript code...
          Hide
          Petr Škoda added a comment -

          I am going to do a bit testing later today and I will try to remember more of the mod_resource history...

          Show
          Petr Škoda added a comment - I am going to do a bit testing later today and I will try to remember more of the mod_resource history...
          Hide
          Petr Škoda added a comment -

          I think this is a nice example why we should use iframe instead of object tag, it is the good old way that is now again recommended in HTML5 (default doctype since Moodle 2.4). The following patch seemed to work fine for me:

          https://github.com/skodak/moodle/compare/009f190...wip_MDL-30769_m25_iframe

          Show
          Petr Škoda added a comment - I think this is a nice example why we should use iframe instead of object tag, it is the good old way that is now again recommended in HTML5 (default doctype since Moodle 2.4). The following patch seemed to work fine for me: https://github.com/skodak/moodle/compare/009f190...wip_MDL-30769_m25_iframe
          Hide
          Rossiani Wijaya added a comment -

          Hi Petr,

          Thank you for the feedback and patch.

          Patch looks great and I backported to 2.4 and 2.3.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Hi Petr, Thank you for the feedback and patch. Patch looks great and I backported to 2.4 and 2.3. Submitting for integration review.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Petr Škoda added a comment -

          Hello, I am not sure if it is ok to backport to 2.3 because it is still supposed to be XHTML Strict 1.0. Thanks anyway.

          Show
          Petr Škoda added a comment - Hello, I am not sure if it is ok to backport to 2.3 because it is still supposed to be XHTML Strict 1.0. Thanks anyway.
          Hide
          Dan Poltawski added a comment -

          Ok, integrated to master and 24 only.

          While it would be nice to have a solution for 2.3, I really do not think it is worth spending any more time on this issue. There are bigger problems to solve.

          Show
          Dan Poltawski added a comment - Ok, integrated to master and 24 only. While it would be nice to have a solution for 2.3, I really do not think it is worth spending any more time on this issue. There are bigger problems to solve.
          Hide
          David Monllaó added a comment -

          It passes, tested in 24 and master

          Show
          David Monllaó added a comment - It passes, tested in 24 and master
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: