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.

Sign up for GitHub

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

Jump to bottom

Avoid stuff before <!DOCTYPE> #9891

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Avoid stuff before <!DOCTYPE> #9891

wants to merge 3 commits into from

Conversation

manu0401
Copy link
Contributor

@manu0401 manu0401 commented Feb 19, 2025 •
edited
Loading

This change makes sure the page component are output in proper order: JS includes in <head>, and page content inside <body>. Nothing should appear before <!DOCTYPE>

If accepted, the same change should be done to most files in the view directory.

@manu0401
Avoid stuff before <!DOCTYPE>
44ab5fa 
This change makes sure the page component are output in proper
order: JS includes in <head>, and page content inside <body>.
Nothing should appear before <!DOCTYPE>

If accepted, the same change should be done to most files in
the view directory.
@DanielnetoDotCom
Copy link
Member

thanks,

I agree with the users.php

but I am not container_fluid_footer.php and view/container_fluid_header.php

@manu0401
Copy link
Contributor Author

but I am not container_fluid_footer.php and view/container_fluid_header.php

I changed it to just include the HTML bits instead of using php to echo them. Was this your concern, or do you want to get this included using another approach? (Which one?)

@DanielnetoDotCom
Copy link
Member

I do not see why to add the <div class="container-fluid">

@manu0401
Copy link
Contributor Author

I fixed more files, but there are cases where large chunks of HTML are output before $_Page->print().

We could add $_Page->setIncludeInHeadStr() and setIncludeInBodyStr() to add an a verbatim string instead of a file?

@Maikuolan
Copy link
Contributor

Out of curiousity, where in the AVideo codebase exactly is includeInBody being used?

I see that this pull request adds some calls to the setIncludeInBody method, and I see that the setIncludeInBody method populates the includeInBody property. But, searching the codebase for the includeInBody property, I don't see where it's actually being used right now (the search results return only where the property is first defined, and then some occurrences of setIncludeInBody). Based on its name, I can surmise it's purpose and what it's supposed to do, but I don't see where exactly that's happening.

@Maikuolan
Copy link
Contributor

Also, based on the method's signature, its supplied parameter should be an array, but the calls to it in this pull request supply a string. Not sure whether that'll be a problem or not, but figured worth mentioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@manu0401 @DanielnetoDotCom @Maikuolan