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

          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: