Moodle
  1. Moodle
  2. MDL-38794

Horizontal scroll bar not present in Book

    Details

    • Rank:
      48856

      Description

      We noticed that chapters in Books that include wide content (e.g., two images side by side, each in their own table cell), there are no horizontal scroll bars. If the browser window is narrowed to the point where you can't see both images, a horizontal scroll does not appear. It's therefore impossible to view the entire chapter unless you expand the browser (in Moodle 1.9.18, for the same chapter, we do see the horizontal scroll bar). Also, this same content, when placed in a Page, a horizontal scroll bar is present. I have attached two images to demonstrate this... one showing the book chapter (with no scroll bar), and the other showing the same content in a page (with a scroll bar). We see this on any stock theme, and all browsers.

        Issue Links

          Activity

          Hide
          Vishal Raheja added a comment -

          Hi!
          This is just a css hack for a particular module. I've just started with providing patches in this community, so I don't know if it is okay with you for this hack is just specific to the book module. It might happen that in some other modules you may face the same problem. Still, it should work for this module at least. Let me know if there are any problems with the patch.

          Show
          Vishal Raheja added a comment - Hi! This is just a css hack for a particular module. I've just started with providing patches in this community, so I don't know if it is okay with you for this hack is just specific to the book module. It might happen that in some other modules you may face the same problem. Still, it should work for this module at least. Let me know if there are any problems with the patch.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that, Brian, and thanks for the patch Vishal.

          Show
          Michael de Raadt added a comment - Thanks for reporting that, Brian, and thanks for the patch Vishal.
          Hide
          Prateek Sachan added a comment - - edited

          Hi Vishal and Michael,

          I've provided a fix for the bug. The hack above wasn't good (I guess). Basically, I passed a 'overflowdiv'=>true parameter condition to the function format_text. It automatically binds the content under .no-overflow class (having overflow: auto) defined in the CSS. It's working. I checked it both on the master and 24 branch. You may have to refresh to clear the cache and view changes. I've added the patch for MDL-38850 also.

          master: https://github.com/prateeksachan/moodle/compare/MDL-38794
          MOODLE_24_STABLE : https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38794_24

          Show
          Prateek Sachan added a comment - - edited Hi Vishal and Michael, I've provided a fix for the bug. The hack above wasn't good (I guess). Basically, I passed a 'overflowdiv'=>true parameter condition to the function format_text . It automatically binds the content under .no-overflow class (having overflow: auto ) defined in the CSS. It's working. I checked it both on the master and 24 branch. You may have to refresh to clear the cache and view changes. I've added the patch for MDL-38850 also. master: https://github.com/prateeksachan/moodle/compare/MDL-38794 MOODLE_24_STABLE : https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38794_24
          Hide
          Prateek Sachan added a comment -

          Also, at this point I would like to point out that I found some mod files that didn't pass the parameter overflowdiv=>true while returning content via function _format_text_due to which similar problem may arise in the future with some other modules.

          Show
          Prateek Sachan added a comment - Also, at this point I would like to point out that I found some mod files that didn't pass the parameter overflowdiv=>true while returning content via function _format_text_due to which similar problem may arise in the future with some other modules.
          Hide
          Aparup Banerjee added a comment - - edited

          this looks good to me. Thanks for using the API

          Prateek, feel free to provide a patch here that fixes these other areas too.

          edit: do remember to add test instructions for all the areas too. The test instructions will be useful when creating automated tests later. (in this case i think it would be a behat test)

          Show
          Aparup Banerjee added a comment - - edited this looks good to me. Thanks for using the API Prateek, feel free to provide a patch here that fixes these other areas too. edit: do remember to add test instructions for all the areas too. The test instructions will be useful when creating automated tests later. (in this case i think it would be a behat test)
          Hide
          Prateek Sachan added a comment - - edited

          Thanks Aparup. . I was really exhilarated after fixing this particular issue.(It was a major one!!).

          Testing Instructions:
          1. Add a new Resource -> Book.
          2. Add a new chapter.
          3. Insert a large image in "content" for the chapter.
          4. Save the chapter and view it.

          Actual Result: There is no horizontal scroll bar at the bottom of the image.
          Expected Result: There should be a horizontal scroll bar at the bottom of the image enabling users to see the image properly.

          edit: I'll definitely report issues and provide fixing patches for them as well, if I encounter this problem with other resources.

          Show
          Prateek Sachan added a comment - - edited Thanks Aparup. . I was really exhilarated after fixing this particular issue.(It was a major one!!). Testing Instructions: 1. Add a new Resource -> Book. 2. Add a new chapter. 3. Insert a large image in "content" for the chapter. 4. Save the chapter and view it. Actual Result: There is no horizontal scroll bar at the bottom of the image. Expected Result: There should be a horizontal scroll bar at the bottom of the image enabling users to see the image properly. edit: I'll definitely report issues and provide fixing patches for them as well, if I encounter this problem with other resources.
          Hide
          Ankit Agarwal added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Ankit Agarwal added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
          Hide
          Colin Matheson added a comment -

          We encountered this issue in 2.5.1 (as we have any people create powerpoints, save the slides as jpgs, and insert them into books) and changed the line mentioned above https://github.com/prateeksachan/moodle/compare/MDL-38794 and it resolved the issue. I am curious why this simple change has not been incorporated into the official code?

          Show
          Colin Matheson added a comment - We encountered this issue in 2.5.1 (as we have any people create powerpoints, save the slides as jpgs, and insert them into books) and changed the line mentioned above https://github.com/prateeksachan/moodle/compare/MDL-38794 and it resolved the issue. I am curious why this simple change has not been incorporated into the official code?
          Hide
          Ankit Agarwal added a comment -

          Hi Prateek,
          The patch looks good to me. Pushing forward for integration.
          Thanks

          Show
          Ankit Agarwal added a comment - Hi Prateek, The patch looks good to me. Pushing forward for integration. Thanks
          Hide
          Sam Hemelryk added a comment -

          Thanks Prateek this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Prateek this has been integrated now.
          Hide
          Petr Škoda added a comment -

          works fine for me, thanks

          Show
          Petr Škoda added a comment - works fine for me, thanks
          Hide
          Damyon Wiese added a comment -

          This issue along with 77 of it's friends has been sent upstream and released to the world.

          Thankyou for your contribution.

          Show
          Damyon Wiese added a comment - This issue along with 77 of it's friends has been sent upstream and released to the world. Thankyou for your contribution.

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: