Moodle
  1. Moodle
  2. MDL-29004

Wiki: printerfriendly view don't show / print images uploaded via html-editor.

    Details

    • Rank:
      18549

      Description

      In wiki: It seems that "printerfriendly view" / print don't show images inserted with use of html-editor's image-icon.

      URl in printerfriendly view (broken):
      /mod/wiki/@@PLUGINFILE@@/imagefilename.png

      URL (example) in wiki normal view (ok):
      /pluginfile.php/6701/mod_wiki/attachments/121/imagefilename.png

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that. I was able to replicate the problem.

        Show
        Michael de Raadt added a comment - Thanks for reporting that. I was able to replicate the problem.
        Hide
        R. Müller added a comment -

        I encountred the same problem with moodle 2.2.2 with linux and mysql. Is there already a fix or a workaround available?

        Thanks for your response.

        cheers,
        Reto

        Show
        R. Müller added a comment - I encountred the same problem with moodle 2.2.2 with linux and mysql. Is there already a fix or a workaround available? Thanks for your response. cheers, Reto
        Hide
        Tomasz Muras added a comment -

        This is still unresolved in the 2.4. I don't think there is a work-around.

        Show
        Tomasz Muras added a comment - This is still unresolved in the 2.4. I don't think there is a work-around.
        Hide
        Tomasz Woźniak added a comment -

        There is patch fixed this problem:

        https://github.com/enovation/moodle/compare/MDL-29004

        Show
        Tomasz Woźniak added a comment - There is patch fixed this problem: https://github.com/enovation/moodle/compare/MDL-29004
        Hide
        Brian King added a comment -

        I've tested this and it works. I commented on https://github.com/enovation/moodle/compare/MDL-29004, suggesting that the fix be moved into wiki_parse_content() (which also works, and avoids re-getting the course module and context).

        Show
        Brian King added a comment - I've tested this and it works. I commented on https://github.com/enovation/moodle/compare/MDL-29004 , suggesting that the fix be moved into wiki_parse_content() (which also works, and avoids re-getting the course module and context).
        Hide
        Tomasz Woźniak added a comment -

        @Brian King: yes, your suggestion is good. It is better when we change urls into wiki_parse_content(). I've changed our code.

        Show
        Tomasz Woźniak added a comment - @Brian King: yes, your suggestion is good. It is better when we change urls into wiki_parse_content(). I've changed our code.
        Hide
        Adrian Greeve added a comment -

        Hello,

        The code looks good, I just have some very minor things to mention:

        When you've fixed up those issues please submit for integration, or let me know and I'll submit it for you.

        Thanks.

        Show
        Adrian Greeve added a comment - Hello, The code looks good, I just have some very minor things to mention: Line 601 is quite long. See http://docs.moodle.org/dev/Coding_style#Maximum_Line_Length All of the commit messages should follow the {MDL-XXXX component: commit message format. See http://docs.moodle.org/dev/Peer_reviewing_checklist#Git When you've fixed up those issues please submit for integration, or let me know and I'll submit it for you. Thanks.
        Hide
        Tomasz Woźniak added a comment -

        Line 601 is trimmed.

        Show
        Tomasz Woźniak added a comment - Line 601 is trimmed.
        Hide
        Brian King added a comment -

        Tomasz,

        I think you'll need to provide all the changes in a commit with a name like 'MDL-29004 Wiki: display images in printer-friendly view' before this can get integrated ...

        Cheers

        Show
        Brian King added a comment - Tomasz, I think you'll need to provide all the changes in a commit with a name like ' MDL-29004 Wiki: display images in printer-friendly view' before this can get integrated ... Cheers
        Hide
        Tomasz Muras added a comment -

        Yeah, we'll need to change the commit comments. Do you also mean that you want 1 single commit?

        Show
        Tomasz Muras added a comment - Yeah, we'll need to change the commit comments. Do you also mean that you want 1 single commit?
        Hide
        Brian King added a comment -

        I just thought it might be easier and cleaner to take the diff of those commits, patch a new branch with it, and update the links in the tracker to point to the new branch. Personally, I just want to see it integrated, I don't care how many commits there are .

        Show
        Brian King added a comment - I just thought it might be easier and cleaner to take the diff of those commits, patch a new branch with it, and update the links in the tracker to point to the new branch. Personally, I just want to see it integrated, I don't care how many commits there are .
        Hide
        Adrian Greeve added a comment -

        Hello Tomasz,

        • I think that all of the commits should be squashed into one single commit.
        • The commit message that Brian wrote would be perfect (with multiple commits, each line must follow the same format).
        • Also, it looks like you have introduced some white space on line 604. If you could also remove that, then integration should be smoother.

        Thanks again.

        Show
        Adrian Greeve added a comment - Hello Tomasz, I think that all of the commits should be squashed into one single commit. The commit message that Brian wrote would be perfect (with multiple commits, each line must follow the same format). Also, it looks like you have introduced some white space on line 604. If you could also remove that, then integration should be smoother. Thanks again.
        Hide
        Didier Raboud added a comment -

        So as to remove friction for the integration of this fix, I have squashed all commits in one, used Brian's commit message and dropped the introduction of whit space on line 604. I have also rebased it on latest master:

        https://github.com/OdyX/moodle/commit/ef33be9556b68aa76bc03edbb3b31dcd53c56c04

        Thanks in advance, cheers,
        OdyX

        Show
        Didier Raboud added a comment - So as to remove friction for the integration of this fix, I have squashed all commits in one, used Brian's commit message and dropped the introduction of whit space on line 604. I have also rebased it on latest master: https://github.com/OdyX/moodle/commit/ef33be9556b68aa76bc03edbb3b31dcd53c56c04 Thanks in advance, cheers, OdyX
        Hide
        Adrian Greeve added a comment -

        Thanks Didier,

        Putting up for integration review.

        Show
        Adrian Greeve added a comment - Thanks Didier, Putting up for integration review.
        Hide
        Damyon Wiese added a comment -

        Thanks for the patch - unfortunately I don't think this has been fixed in the right way.

        The call to rewrite_pluginfile_urls should be made in the print_pretty_view function of page_wiki_prettyview. Doing it in only if 'printable' is set in "wiki_parse_content" is inconsistent and confusing.

        Also - I'm not sure why you are deleting the open div tag from mod/wiki print_pretty_view - but the result is unbalanced divs and I don't think it should be done at all.

        Also - as this is a bug fix it would be nice if it had backport branches...

        Show
        Damyon Wiese added a comment - Thanks for the patch - unfortunately I don't think this has been fixed in the right way. The call to rewrite_pluginfile_urls should be made in the print_pretty_view function of page_wiki_prettyview. Doing it in only if 'printable' is set in "wiki_parse_content" is inconsistent and confusing. Also - I'm not sure why you are deleting the open div tag from mod/wiki print_pretty_view - but the result is unbalanced divs and I don't think it should be done at all. Also - as this is a bug fix it would be nice if it had backport branches...
        Hide
        Damyon Wiese added a comment -

        Thanks for the patch - it just needs a bit more work before we can integrating it. I'm reopening the issue for this week.

        Show
        Damyon Wiese added a comment - Thanks for the patch - it just needs a bit more work before we can integrating it. I'm reopening the issue for this week.
        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
        Brian King added a comment -

        Providing an updated patch based on Damyon's comments.

        This is actually Tomasz' original fix, with a compliant commit message and no extra whitespace.

        Thanks Tomasz!

        Show
        Brian King added a comment - Providing an updated patch based on Damyon's comments. This is actually Tomasz' original fix, with a compliant commit message and no extra whitespace. Thanks Tomasz!
        Hide
        Adrian Greeve added a comment -

        It looks like the issues that Damyon had raised have been addressed.
        Moving this back to integration.

        Show
        Adrian Greeve added a comment - It looks like the issues that Damyon had raised have been addressed. Moving this back to integration.
        Hide
        Damyon Wiese added a comment -

        Thanks for the updated branches. This looks good to me and does the job (tested on master). Integrated to master, 25 and 24.

        Show
        Damyon Wiese added a comment - Thanks for the updated branches. This looks good to me and does the job (tested on master). Integrated to master, 25 and 24.
        Hide
        Rajesh Taneja added a comment -

        While creating a new wiki page, I got following error (This is not happening on master)

        Did you remember to call setType() for 'groupinfo'? Defaulting to PARAM_RAW cleaning.
        
            line 1305 of /lib/formslib.php: call to debugging()
            line 281 of /lib/formslib.php: call to moodleform->detectMissingSetType()
            line 202 of /lib/formslib.php: call to moodleform->_process_submission()
            line 906 of /mod/wiki/pagelib.php: call to moodleform->moodleform()
            line 105 of /mod/wiki/create.php: call to page_wiki_create->set_action()
        
        This page did not call $PAGE->set_url(...). Using http://rajesh.moodle.local/im/mod/wiki/create.php?wid=4&group=0&uid=0&title=first%20page
        
            line 558 of /lib/pagelib.php: call to debugging()
            line 734 of /lib/pagelib.php: call to moodle_page->magic_get_url()
            line 496 of /mod/wiki/lib.php: call to moodle_page->__get()
            line 2042 of /lib/navigationlib.php: call to wiki_extend_navigation()
            line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity()
            line 222 of /blocks/navigation/block_navigation.php: call to global_navigation->initialise()
            line 178 of /blocks/navigation/block_navigation.php: call to block_navigation->get_navigation()
            line 296 of /blocks/moodleblock.class.php: call to block_navigation->get_content()
            line 238 of /blocks/moodleblock.class.php: call to block_base->formatted_contents()
            line 956 of /lib/blocklib.php: call to block_base->get_content_for_output()
            line 1008 of /lib/blocklib.php: call to block_manager->create_block_contents()
            line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created()
            line 3202 of /lib/outputrenderers.php: call to block_manager->region_has_content()
            line 3238 of /lib/outputrenderers.php: call to core_renderer->body_css_classes()
            line 26 of /theme/clean/layout/embedded.php: call to core_renderer->body_attributes()
            line 850 of /lib/outputrenderers.php: call to include()
            line 780 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
            line 723 of /lib/outputrenderers.php: call to core_renderer->header()
            line 2607 of /lib/weblib.php: call to core_renderer->redirect_message()
            line 118 of /mod/wiki/create.php: call to redirect()
        
        id="page-mod-wiki-create" class="format-topics path-mod path-mod-wiki gecko dir-ltr lang-en yui-skin-sam yui3-skin-sam rajesh-moodle-local--im pagelayout-redirect course-34 context-316 cmid-86 category-1 editing has-region-side-pre used-region-side-pre has-region-side-post empty-region-side-post"> 
        

        I don't think this is related, but will wait for response before passing it.

        Show
        Rajesh Taneja added a comment - While creating a new wiki page, I got following error (This is not happening on master) Did you remember to call setType() for 'groupinfo'? Defaulting to PARAM_RAW cleaning. line 1305 of /lib/formslib.php: call to debugging() line 281 of /lib/formslib.php: call to moodleform->detectMissingSetType() line 202 of /lib/formslib.php: call to moodleform->_process_submission() line 906 of /mod/wiki/pagelib.php: call to moodleform->moodleform() line 105 of /mod/wiki/create.php: call to page_wiki_create->set_action() This page did not call $PAGE->set_url(...). Using http://rajesh.moodle.local/im/mod/wiki/create.php?wid=4&group=0&uid=0&title=first%20page line 558 of /lib/pagelib.php: call to debugging() line 734 of /lib/pagelib.php: call to moodle_page->magic_get_url() line 496 of /mod/wiki/lib.php: call to moodle_page->__get() line 2042 of /lib/navigationlib.php: call to wiki_extend_navigation() line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity() line 222 of /blocks/navigation/block_navigation.php: call to global_navigation->initialise() line 178 of /blocks/navigation/block_navigation.php: call to block_navigation->get_navigation() line 296 of /blocks/moodleblock.class.php: call to block_navigation->get_content() line 238 of /blocks/moodleblock.class.php: call to block_base->formatted_contents() line 956 of /lib/blocklib.php: call to block_base->get_content_for_output() line 1008 of /lib/blocklib.php: call to block_manager->create_block_contents() line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created() line 3202 of /lib/outputrenderers.php: call to block_manager->region_has_content() line 3238 of /lib/outputrenderers.php: call to core_renderer->body_css_classes() line 26 of /theme/clean/layout/embedded.php: call to core_renderer->body_attributes() line 850 of /lib/outputrenderers.php: call to include() line 780 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 723 of /lib/outputrenderers.php: call to core_renderer->header() line 2607 of /lib/weblib.php: call to core_renderer->redirect_message() line 118 of /mod/wiki/create.php: call to redirect() id="page-mod-wiki-create" class="format-topics path-mod path-mod-wiki gecko dir-ltr lang-en yui-skin-sam yui3-skin-sam rajesh-moodle-local--im pagelayout-redirect course-34 context-316 cmid-86 category-1 editing has-region-side-pre used-region-side-pre has-region-side-post empty-region-side-post"> I don't think this is related, but will wait for response before passing it.
        Hide
        Rajesh Taneja added a comment -

        Above error appears only on integration master and not on any other branch.

        Show
        Rajesh Taneja added a comment - Above error appears only on integration master and not on any other branch.
        Hide
        Rajesh Taneja added a comment -

        Sorry for the noise guys, I can't reproduce this anymore.

        Passing this.

        Show
        Rajesh Taneja added a comment - Sorry for the noise guys, I can't reproduce this anymore. Passing this.
        Hide
        Damyon Wiese added a comment -

        There was a young man named McGee
        Who thought squashing bugs was easy
        He tried it one day
        And to his dismay
        The bug guts made his keyboard all greasy

        Thanks!

        This has issue has been fixed and released in Moodle.

        Show
        Damyon Wiese added a comment - There was a young man named McGee Who thought squashing bugs was easy He tried it one day And to his dismay The bug guts made his keyboard all greasy Thanks! This has issue has been fixed and released in Moodle.

          People

          • Votes:
            10 Vote for this issue
            Watchers:
            17 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: