Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix html structure #77

Closed
wants to merge 1 commit into from
Closed

fix html structure #77

wants to merge 1 commit into from

Conversation

t-schroeder
Copy link

@t-schroeder t-schroeder commented Aug 3, 2021

There are too many closing tags for the "qtext" div. The consequence is that when attempting a quiz the page layout is broken.

I can create an issue and attach a screenshot, sample question if necessary.

there were too many closing divs
@marcusgreen
Copy link
Owner

marcusgreen commented Aug 3, 2021

Could you give me steps to reproduce this. I have a unit test to check the divs are evenly closed here

public function test_matching_divs() {

I was addressing something like this in this recent code
#76

$questiontext = html_writer::empty_tag('div', array('class' => 'qtext'));
$questiontext = html_writer::start_div('qtext');
Copy link
Author

@t-schroeder t-schroeder Aug 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is just to get rid of the / in the div start tag. See W3C validator:
image

Edit: The W3C validator says the same problem is there for the "answeroptions" div.

@t-schroeder
Copy link
Author

Nevermind, I just realized I was using version 2021063001 of your plugin. With your fix for #76 you already fixed the problem I was trying to fix here.

The empty div tags for "qtext" and "answeroptions" (which should be start tags instead) still technically create wrong HTML, but seems like most browsers are robus enough to handle that.

@t-schroeder t-schroeder closed this Aug 4, 2021
@marcusgreen
Copy link
Owner

marcusgreen commented Aug 4, 2021

When I worked on that I was looking for a tool to add proper html validation into the unit tests, but (surprisingly) I couldn't find anything, so settled on my crude checking that there was an even number of start and end divs. I would rather my code put out correct HTML even if browsers do tolerate errors. I appreciate your input and will look at your other issue as soon as I can.
Update: I have just asked about this in the moodle developers chat on Telegram.

@t-schroeder t-schroeder deleted the fix-html-structure branch August 4, 2021 15:54
@marcusgreen
Copy link
Owner

I have been running the rendered output through this validator
https://validator.w3.org/nu/#textarea
And it is a bit of a mess. I will work on ensuring the output is more compliant in future versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants