Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29004

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

    Details

    • Database:
      Microsoft SQL
    • Testing Instructions:
      Hide

      Test:
      1. Create a wiki -> wikipage (html-format, no camelcase).
      2. Insert image with image-icon in html-editor.
      3. Save.
      4. Click "Printerfriendly view".
      5. Are image visible or broken?

      Show
      Test: 1. Create a wiki -> wikipage (html-format, no camelcase). 2. Insert image with image-icon in html-editor. 3. Save. 4. Click "Printerfriendly view". 5. Are image visible or broken?
    • Workaround:
      Hide

      Only link to images on web - but that's really not sutiable for ordinary users.

      Show
      Only link to images on web - but that's really not sutiable for ordinary users.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      mdl26_MDL-29004_wiki-fix-images-in-printer-friendly-view

      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

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

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

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting that. I was able to replicate the problem.
            Hide
            mueller.r 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
            mueller.r 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
            tmuras Tomasz Muras added a comment -

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

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

            There is patch fixed this problem:

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

            Show
            twozniak Tomasz Woźniak added a comment - There is patch fixed this problem: https://github.com/enovation/moodle/compare/MDL-29004
            Hide
            brianking 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
            brianking 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
            twozniak 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
            twozniak 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
            abgreeve 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
            abgreeve 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
            twozniak Tomasz Woźniak added a comment -

            Line 601 is trimmed.

            Show
            twozniak Tomasz Woźniak added a comment - Line 601 is trimmed.
            Hide
            brianking 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
            brianking 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
            tmuras 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
            tmuras 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
            brianking 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
            brianking 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
            abgreeve 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
            abgreeve 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
            odyx 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
            odyx 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
            abgreeve Adrian Greeve added a comment -

            Thanks Didier,

            Putting up for integration review.

            Show
            abgreeve Adrian Greeve added a comment - Thanks Didier, Putting up for integration review.
            Hide
            damyon 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 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 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 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 CiBoT added a comment -

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

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            brianking 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
            brianking 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
            abgreeve Adrian Greeve added a comment -

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

            Show
            abgreeve Adrian Greeve added a comment - It looks like the issues that Damyon had raised have been addressed. Moving this back to integration.
            Hide
            damyon 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 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
            rajeshtaneja 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
            rajeshtaneja 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
            rajeshtaneja Rajesh Taneja added a comment -

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

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

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

            Passing this.

            Show
            rajeshtaneja Rajesh Taneja added a comment - Sorry for the noise guys, I can't reproduce this anymore. Passing this.
            Hide
            damyon 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 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:
                  Fix Release Date:
                  9/Sep/13