Moodle
  1. Moodle
  2. MDL-41473

Book HTML zip import incorrect URL produced for internal linking to other chapters

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: Book
    • Labels:
    • Testing Instructions:
      Hide

      1) Create a new book module and save and display.
      2) Progress to the import chapter page (Administration > Import chapter).
      3) Use the attached bookimporttest.zip file as the Zip file, and leave the Type as the default Each Html file represents one chapter.
      4) Once the import processing has completed view the first of the two pages (named a.html and b.html). There is a link on each page to the other page. Without this patch the links are broken (though the addition of 'id' in the url will correct them).

      Show
      1) Create a new book module and save and display. 2) Progress to the import chapter page (Administration > Import chapter). 3) Use the attached bookimporttest.zip file as the Zip file, and leave the Type as the default Each Html file represents one chapter. 4) Once the import processing has completed view the first of the two pages (named a.html and b.html). There is a link on each page to the other page. Without this patch the links are broken (though the addition of 'id' in the url will correct them).
    • Workaround:
      Hide

      Do not use links in html files that you are using to import to a book

      Show
      Do not use links in html files that you are using to import to a book
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull 2.5 Branch:
      wip-MDL-41473-MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-41473-master
    • Rank:
      52513

      Description

      Scenario:
      HTML file A has a relative link to HTML file B. Both files are added to a Zip. The Zip is imported into a Book and chapters are created from the HTML files. The Book module attempts to preserve the link from A to B, using the new paths created as A and B are now chapters in the book. However, there is an error in the link created as it uses 'chapter' as a parameter instead of 'chapterid'.

      Solution:

      In mod/book/tool/importhtml/locallib.php

      In function toolbook_importhtml_import_chapters

      Line 140/141

      Change

      $newcontent = str_replace($match, 'href="'.new moodle_url('/mod/book/view.php',
      array('id'=>$context->instanceid, 'chapter'=>$target->id)).'"', $newcontent);

      to

      $newcontent = str_replace($match, 'href="'.new moodle_url('/mod/book/view.php',
      array('id'=>$context->instanceid, 'chapterid'=>$target->id)).'"', $newcontent);

      Sorry, I'm just an occasional user so not used to creating patch files myself. Hope it helps!

        Activity

        Hide
        Ankit Agarwal added a comment -

        Thanks for reporting this.

        I've put that on the backlog.

        Show
        Ankit Agarwal added a comment - Thanks for reporting this. I've put that on the backlog.
        Hide
        John Beedell added a comment -

        I have allocated myself this bug, as it is important for us at the Open University for this to be fixed.

        Sorry, I do not have access to the edit button to be able to add testing instructions, so they are here:

        1) Create a new book module and save and display.
        2) Progress to the import chapter page (Administration > Import chapter).
        3) Use the attached bookimporttest.zip file as the Zip file, and leave the Type as the default Each Html file represents one chapter.
        4) Once the import processing has completed view the first of the two pages (named a.html and b.html). There is a link on each page to the other page. Without this patch the links are broken (though the addition of 'id' in the url will correct them).

        Show
        John Beedell added a comment - I have allocated myself this bug, as it is important for us at the Open University for this to be fixed. Sorry, I do not have access to the edit button to be able to add testing instructions, so they are here: 1) Create a new book module and save and display. 2) Progress to the import chapter page (Administration > Import chapter). 3) Use the attached bookimporttest.zip file as the Zip file, and leave the Type as the default Each Html file represents one chapter. 4) Once the import processing has completed view the first of the two pages (named a.html and b.html). There is a link on each page to the other page. Without this patch the links are broken (though the addition of 'id' in the url will correct them).
        Hide
        Ankit Agarwal added a comment -

        Thanks Matt and John for finding and fixing this bug.

        The patch looks perfect. Just one minor thing.
        We recommend commit message in following format:-
        MDL-41473 book: short description of the patch

        If you can just update your branches with this format, I can push the patch forward for integration.

        Thanks

        Show
        Ankit Agarwal added a comment - Thanks Matt and John for finding and fixing this bug. The patch looks perfect. Just one minor thing. We recommend commit message in following format:- MDL-41473 book: short description of the patch If you can just update your branches with this format, I can push the patch forward for integration. Thanks
        Hide
        John Beedell added a comment -

        Sorry Ankit, I should read the guidance more carefully.

        I've updated my github branches with corrected commit message.

        Many thanks.

        Show
        John Beedell added a comment - Sorry Ankit, I should read the guidance more carefully. I've updated my github branches with corrected commit message. Many thanks.
        Hide
        Ankit Agarwal added a comment -

        Thanks for updating the patch, pushing forward.

        Show
        Ankit Agarwal added a comment - Thanks for updating the patch, pushing forward.
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        John Beedell added a comment -

        My github is now updated.

        Show
        John Beedell added a comment - My github is now updated.
        Hide
        Damyon Wiese added a comment -

        Thanks, this was integrated to 24, 25 and master.

        I had to cherrypick the patch because the dev branches contained merges. Please use git rebase instead of git pull when rebasing against master.

        e.g.

        git rebase origin master

        (replay all my work on top of the latest origin/master).

        Show
        Damyon Wiese added a comment - Thanks, this was integrated to 24, 25 and master. I had to cherrypick the patch because the dev branches contained merges. Please use git rebase instead of git pull when rebasing against master. e.g. git rebase origin master (replay all my work on top of the latest origin/master).
        Hide
        Sam Hemelryk added a comment -

        Thanks guys - tested and passed

        Show
        Sam Hemelryk added a comment - Thanks guys - tested and passed
        Hide
        Dan Poltawski added a comment -

        Congratulations - this issue has been included in Moodle and is now available on our git mirrors and shortly will become available on the download servers shortly.

        Show
        Dan Poltawski added a comment - Congratulations - this issue has been included in Moodle and is now available on our git mirrors and shortly will become available on the download servers shortly.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: