Moodle
  1. Moodle
  2. MDL-31311

Fix duplicate title attributes for iframed file and URL resources

    Details

    • Testing Instructions:
      Hide

      1. Make sure that iframes are selected as an option under Site Administration --> Plugins --> Activity Modules --> URL and Site Administration --> Plugins --> Activity Modules --> File
      2. Navigate to a course, turn editing on, add a new File or URL resource.
      3. Fill in data and select "In Frame" from the Display drop down
      4. Save the resource and click to view it.
      5. View the source and see that the title attribute for both iframe elements is identical and only reflects the module name ('File' or 'URL')

      Show
      1. Make sure that iframes are selected as an option under Site Administration --> Plugins --> Activity Modules --> URL and Site Administration --> Plugins --> Activity Modules --> File 2. Navigate to a course, turn editing on, add a new File or URL resource. 3. Fill in data and select "In Frame" from the Display drop down 4. Save the resource and click to view it. 5. View the source and see that the title attribute for both iframe elements is identical and only reflects the module name ('File' or 'URL')
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-31311-Fix_iframe_titles
    • Rank:
      37779

      Description

      The title attribute for the navigation frame and the content frames on File and URL resources are not helpful in their current state. They are identical and only reflect the type of resource that is being viewed:

      <frameset rows="$framesize,*">
      <frame src="$navurl" title="$modulename"/>
      <frame src="$exteurl" title="$modulename"/>
      </frameset>

      This is confusing to those relying on screen readers.

        Activity

        Hide
        Rajesh Taneja added a comment -

        Thanks for spotting the issue and providing the patch, Glenn.
        I think, it will be nice to leave the resource title unchanged and change the title for resource content.
        i.e.

        $contentframetitle = format_string($resource->name);
        ...
        ...
        
        <frame src="$navurl" title="$modulename" />
        <frame src="$fileurl" title="$contentframetitle" />
        
        Show
        Rajesh Taneja added a comment - Thanks for spotting the issue and providing the patch, Glenn. I think, it will be nice to leave the resource title unchanged and change the title for resource content. i.e. $contentframetitle = format_string($resource->name); ... ... <frame src= "$navurl" title= "$modulename" /> <frame src= "$fileurl" title= "$contentframetitle" />
        Hide
        Glenn Ansley added a comment -

        Hi Rajesh. Thanks for your feedback. I have provided made the changes. Same diff URL / Branch

        Show
        Glenn Ansley added a comment - Hi Rajesh. Thanks for your feedback. I have provided made the changes. Same diff URL / Branch
        Hide
        Rajesh Taneja added a comment -

        Thanks for fixing this Glenn,
        Patch looks spot-on. Please feel free to push it for integration review, or let me know and I will do the needful.

        Show
        Rajesh Taneja added a comment - Thanks for fixing this Glenn, Patch looks spot-on. Please feel free to push it for integration review, or let me know and I will do the needful.
        Hide
        Glenn Ansley added a comment -

        Hi Rajesh,
        Looks like I'm either missing the ability or the knowhow to submit it for integration review. Looks like you'll have to do it.

        Show
        Glenn Ansley added a comment - Hi Rajesh, Looks like I'm either missing the ability or the knowhow to submit it for integration review. Looks like you'll have to do it.
        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
        Glenn Ansley added a comment -

        Rebased

        Show
        Glenn Ansley added a comment - Rebased
        Hide
        Aparup Banerjee added a comment -

        just adding to the repository field here.

        Show
        Aparup Banerjee added a comment - just adding to the repository field here.
        Hide
        Aparup Banerjee added a comment -

        thanks, this improvement is in integration now for testing.

        Show
        Aparup Banerjee added a comment - thanks, this improvement is in integration now for testing.
        Hide
        Adrian Greeve added a comment -

        Checked out the solution pre and post patch. The title reflects the resource better.
        Thanks.

        Show
        Adrian Greeve added a comment - Checked out the solution pre and post patch. The title reflects the resource better. Thanks.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories.

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for your collaboration, this code has been integrated upstream and it's available in all the repositories. Closing, ciao

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: