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

div inside p #416

Open
u07 opened this issue Oct 28, 2020 · 6 comments
Open

div inside p #416

u07 opened this issue Oct 28, 2020 · 6 comments

Comments

@u07
Copy link

u07 commented Oct 28, 2020

Hi! There is a code in dw2pdf's renderer.php:

    /**
     * Wrap centered media in a div to center it
     */
    function _media($src, $title = NULL, $align = NULL, $width = NULL,
                    $height = NULL, $cache = NULL, $render = true) {

        $out = '';
        if($align == 'center') {
            $out .= '<div align="center" style="text-align: center">';
        }

        $out .= parent::_media($src, $title, $align, $width, $height, $cache, $render);

        if($align == 'center') {
            $out .= '</div>';
        }

        return $out;
    }

This results in html code like this:

<p>
<a href="1.jpg" class="media" title="1.jpg"><div align="center" style="text-align: center"><img src="1.jpg" class="mediacenter" title="1.jpg" alt="1.jpg" /></div></a>
</p>

Here we have a <DIV> tag inside <A> tag inside <P> tag, which does not comply HTML standards: a P tag should contain phrasing content only. Also it breaks mpdf logic and makes links in PDF broken, so, it's impossible to click on the image. You know, in DokuWiki images are links to their full versions. So, in PDF right- or left-aligned images are links, and center-aligned are not .

Is it possible to rewrite this function to align images somehow else, without introducing new DIVs inside <a>?

image

@u07

This comment has been minimized.

@u07
Copy link
Author

u07 commented Oct 29, 2020

Okay, I've managed to patch it. Do you accept patches or pull requests, what's the right way to send it? or just here, in comments? I've never made pull requests :)

@Klap-in
Copy link
Collaborator

Klap-in commented Oct 29, 2020

Is this issue also connected to #275?
Code example is welcome.

Pull request is welcome as well, but I'm wondering if it is worth to check more thoroughly the image handling.

u07 added a commit to u07/dokuwiki-plugin-dw2pdf that referenced this issue Oct 30, 2020
oops, forgot a comment
@u07
Copy link
Author

u07 commented Oct 30, 2020

Is this issue also connected to #275?

They are not. That patch can be applied alongside with this one. Actually №275 helped to notice broken links. It made them visible, like this:

image

Code example

I moved <div> tag outside of <a>. To do so I moved the div wrapping code from _media() to internalmedia() and externalmedia(), cause they actually produce <a> tags.

    // Center-align images with their corresponding links
    function internalmedia($src, $title = null, $align = null, $width = null,
                           $height = null, $cache = null, $linking = null, $return = false) {
        
		$out = parent::internalmedia($src, $title, $align, $width, $height, $cache, $linking, true);
        
		if($align == 'center') {
			$out = '<div align="center" style="text-align: center">' .$out. '</div>';
        }
		
        if($return) return $out;
        else $this->doc .= $out;
    }	

It's impossible to show on a screenshot :) but both images are links now. And #275 produces a nice paragraph flow:

image

@u07
Copy link
Author

u07 commented Nov 3, 2020

Here is a live example with both patches applied: https://petelinsasha.ru/wiki/playground/dw2pdf_test

@Klap-in
Copy link
Collaborator

Klap-in commented Oct 12, 2022

If the html is not proper, that should be 'fixed' before we feed it to mpdf, or mpdf should be improved (then tiny example that exactly replicate the case should be created, and used to open an issue there), or DokuWiki should be changed as well, if it is not proper html.

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

No branches or pull requests

2 participants