better automatic word-break behavior #1949
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use of uninitialized value in pattern match (m//) .. cgi-bin/LJ/CleanHTML.pm line 1419. Use of uninitialized value in concatenation (.) or string .. cgi-bin/DW/Controller/Entry.pm line 1496.
Should be treated the same as the br tag. This fixes the reported problem where if someone used <wbr> in an entry, it would get autoclosed with </wbr>, which is invalid markup. Now it will get changed to <wbr /> instead.
In most areas (entries, subjects, comments) the HTML cleaner will insert a wbr tag in any "word" (text unbroken by whitespace) longer than 40 characters, exactly at the 40th character point, and for every 40 characters thereafter, if autoformatting is active. This behavior could be improved, so let's try checking each 40 characters for punctuation characters, and if found, insert the word break at that point instead.
1. The \B match failed if the word ended with punctuation, so just check to see if we're at the end of the word after the match succeeds. 2. Make sure the regex finds the LAST punctuation character in the string. Unexpectedly, it was matching the first one instead. 3. Don't do a breakpoint shift if the last punctuation character in the string is the first character in the string - a common edge case resulting in a premature word break. 4. Refactor printing logic to remove unneeded else case from conditional. Also: more tests! Tests are great.
|
ooh yay that's been a problem for like 15 years, it just happens rarely enough that nobody ever bothered fixing it...
|
|
My new motto: no problem too rare, no fix too over-engineered. Actually, that's a terrible motto, never mind 😜 |
|
yeah but let's face it it's TOTALLY TRUE
<3
|
zorkian
reviewed
Jan 25, 2017
| is( $orig_post, $clean_post, "Choose last punctuation in string" ); | ||
|
|
||
| $orig_post = qq{"This_is_a_test_of_the_emergency_word_break_system."}; | ||
| $clean_post = qq{"This_is_a_test_of_the_emergency_word_br<wbr />eak_system."}; |
There was a problem hiding this comment.
This is supposed to break mid-word as opposed to post-_?
There was a problem hiding this comment.
I used the underscore character in this test because it's a special character included in \w, and I was making sure the word break wasn't inserted after the initial quotation mark.
There was a problem hiding this comment.
(Although if you don't want underscore treated differently, that's understandable, but I couldn't think of a simple way to overcome that exception.)
There was a problem hiding this comment.
Nope this is fine! I was just curious about the thinking.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now the HTML cleaner will prefer punctuation marks for word breaks, if available, and has been told that </wbr> is not a thing. Includes many tests for the new behavior.
Fixes #1948.