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

Better Footnote title attributes? #387

Open
edent opened this issue Nov 15, 2022 · 11 comments
Open

Better Footnote title attributes? #387

edent opened this issue Nov 15, 2022 · 11 comments

Comments

@edent
Copy link

edent commented Nov 15, 2022

Footnote links' titles can only be set once by the library. This change would make the attribute display the content of the footnote.

How

At the moment, if a user hovers over a footnote, the title attribute displays a tool tip.

For example, if

public string $fn_link_title = "";

is set to "Read the footnote." then all footnotes have this pop-up:

Screenshot of a pop up which says 'read the footnote'


I think it would be nice if it showed users the full text of the footnote. For example:

The pop up now displays the full note


This is controlled by :

$title = $this->fn_link_title;

The change is relatively straightforward:

if ($this->fn_link_title != "") {
	$title = trim( strip_tags( $this->footnotes[$node_id] ) );
	$title = $this->encodeAttribute( $title );
	$attr .= " title=\"$title\"";
}

That takes the text of the footnote, sanitises it, and adds it as the title element.

Would you be interested in a PR for this?

(I have also raised this with WordPress's Jetpack which is running an older version of your library - Automattic/jetpack#27387)

@michelf
Copy link
Owner

michelf commented Nov 15, 2022

How do you propose to handle footnotes like this one1.

Footnotes

  1. Footnotes are often one line of text, but this line of text can contain formatting such as code, emphasis, links, and images such as this one. Actually, it wouldn't surprise me to find that links are pretty common since footnotes are often used for references.

    They can also go quite long with paragraphs and other block-level elements such as:

    tables tables
    rows rows

    blockquotes

    code blocks
    
    • lists
    • and more lists

    So we need a strategy to deal with this. I don't think strip_tags is good enough.

@edent
Copy link
Author

edent commented Nov 15, 2022

A very reasonable question!

To me, it comes down to user needs. I want a tool-tip so I can see whether the footnote says "(Smith, 1996)" or "A funny story about that, back in 1996 I saw ...".

That way, I can decide if I want to visit the footnote.

Links are handled well, I think. echo strip_tags('<a href="https://example.com/">(Smith, 1996)</a>'); spits out (Smith, 1996)

The HTML spec for title doesn't define a maximum length but, anecdotally, some browsers will truncate them.

So I would use strip_tags and then mb_substr() to knock it down to, say, a max of 100±10 characters.

What do you think to that idea?

@michelf
Copy link
Owner

michelf commented Nov 15, 2022

I think truncating it if too long makes this idea reasonable. It should probably truncate at the first end of paragraph </p> and some other tags too (<table>, <blockquote>…). It'd be nice to replace images with the alt text. And spacing is interpreted differently inside title="" than in HTML, so it should probably be somehow normalized (collapsing double-spaces and newlines, then replacing <br/> with actual newlines).

Sorry if this seems a lot of work. Maybe it's not all needed.

I'm not sure if this tooltip should be the default, but at least it should be possible to enable or disable it.

@edent
Copy link
Author

edent commented Nov 16, 2022

Things like alt text will be tricky without either parsing the DOM or using a library like https://github.com/mtibben/html2text

Similarly, stopping at specific tag or replacing <br> with \n might involve a fair bit of parsing.

So, I guess the options are:

  1. Cheat. Strip the tags, Grab the first N characters, and give users a somewhat meaningful preview.
  2. Fully parse the DOM of the generated HTML to grab the appropriate text to a suitable boundary.
  3. Generate a bespoke title from the Markdown directly.

I'm lazy, so happy to contribute a patch for (1). But if you're looking for something more comprehensive, I'll have to bow out.

@michelf
Copy link
Owner

michelf commented Nov 16, 2022

Personally, I think we could cheat a bit more. Something like this:

list($title, $tail) = preg_split('{</p\s*>\s*}i', $title, 2); // split at </p>
$truncated = $tail != '';
$title = preg_replace('{<img\b[^<]\balt\s*=\s*(?:"([^"]*")|(\'([^\']*\')|(\S*)\b)[^<]*>}i', '\1\2\3', $title); // img -> alt text
$title = preg_replace('{\s+}', ' ', $title); // normalize whitespace
$title = trim($title); // remove leading & trailing whitespace
$title = preg_replace('{<br\s*/?>}i', '\n', $title); // br -> \n
$title = strip_tags($title);
$before_truncation = $title;
$title = preg_replace('{^.{0,100}+?\b}', '', $title); // cut after 100 characters, but continue for last word.
if ($truncated || $before_truncation != $title) {
   $title .= '…'; // add ellipsis after truncation
}

There's some situations where the img will be parsed incorrectly (if you include unescaped > in quoted attributes (other than the alt itself). But otherwise it should work... in theory. I didn't test it at all, feel free if you want to.

@edent
Copy link
Author

edent commented Jan 3, 2023

Here's a slightly less regex-y way of doing things using PHP's DOMDocument

$doc = new DOMDocument();
$doc->loadHTML( mb_convert_encoding("<p>fugiat <em>consequuntur</em>. 蘵 <h2>Dolores deleniti</h2> <a href='/home'>quia voluptas</a> unde dolor <img alt='esse perspiciatis voluptas' src='img.jpg' />. <p>Autqui 🇬🇧<strong> corporisrerumfuga</strong> quo a. Fuga voluptatem rerum autem. Beatae dolorem et porro ut culpa. Est ut deserunt in quod non omnis suscipit veritatis.</p>", 'HTML', 'UTF-8' ));

$xp = new DOMXPath( $doc );

//	Replace each element with the text inside it
foreach ( $xp->query('//*/text()') as $node ) {
	$p_text = $doc->createTextNode( $node->wholeText . "\n");
	$node->parentNode->replaceChild( $p_text, $node );
}

//	Replace each <img> with its alt text
foreach ( $xp->query('//img') as $node ) {
	$alt_text = $doc->createTextNode($node->getAttribute('alt') . "\n");
	$node->parentNode->replaceChild($alt_text, $node);
}

//	Replace each <br> with a newline
foreach ( $xp->query('//br') as $node ) {
	$newline = $doc->createTextNode( "\n" );
	$node->parentNode->replaceChild( $newline, $node );
}

//	Get a plaintext representation
$title_text = html_entity_decode( strip_tags( $doc->saveHTML() ) );

//	Trim any whitespace
$title_text = trim( $title_text );

The only problem is splitting the text. Not all multibyte languages (like Chinese) have word boundaries like \b. I'm still having a think about how to solve that.

@michelf
Copy link
Owner

michelf commented Jan 4, 2023

I suppose a solution could be to have two limits: 100 characters + characters until word break, but if no word break is found between the 100th and 120th character you simply cut at 100 (or any other arbitrary length).

// cut after 100 characters, but continue for last word (if less than 20 characters).
$title = preg_replace('{^.{0,100}({1,20)?\b|)}u', '', $title);

I think the u pcre flag will make it consider UTF-8 correctly (not cut in the middle of a code point). It would still be susceptible of breaking things in the middle of grapheme clusters (like for multi-code-point emojis, Korean syllables, combining characters, etc.). Fixing this is going to require something more much more complex, like inspecting character classes I suppose. Cutting only at word breaks keeps things much simpler in comparison.

@edent
Copy link
Author

edent commented Jan 4, 2023

I couldn't get that regex to work - and I'm always a little paranoid about how readable and maintainable they are.

This should accomplish the same thing:

//	Split by space
$parts = explode( " ", $title_text );

//	Add each part to a new string until it is 100 characters long
$title = "";

foreach ( $parts as $part) {
	//	Always add the first part
	if ( mb_strlen( $title ) == 0 ) {
		$title .= $part . " ";
		//	If the first part is a very long string, reduce it to 100 characters
		if ( mb_strlen( $title ) > 100 ) {
			$title = mb_substr( $title, 0, 100 );
			break;
		}
	} else if ( ( mb_strlen( $title ) + mb_strlen( $part ) ) < 100 ) {
		//	Add the next whole word which doesn't take the total length over 100 characters
		$title .= $part . " ";
	} else {
		break;
	}
}

//	Trim any whitespace
$title = trim( $title );

//	If it has been truncated, add an ellipsis
if ( mb_strlen( $title ) < ( mb_strlen( $title_text ) ) ) {
	$title .= "";
}

I've tested this with several long strings and it seems to work. I've also made some encoding changes to the above function so it copes better with multibyte characters.

Let me know if you'd like this as a proper PR.

@edent
Copy link
Author

edent commented Mar 21, 2023

One last bug :-)

$this->footnotes[$node_id] returns hashed values for some elements, as defined in:

protected function _hashHTMLBlocks_inMarkdown($text, $indent = 0,

So, for example, a footnote which is a table is returned as B2B

Is there any way to unhash $this->footnotes[$node_id]?

@michelf
Copy link
Owner

michelf commented Mar 21, 2023

$this->unhash($text) (inherited from the Markdown class)

@edent
Copy link
Author

edent commented Mar 26, 2023

I ended up using formParagraphs() so that markdown inside a title was properly decoded first.

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

No branches or pull requests

2 participants