mojo icon indicating copy to clipboard operation
mojo copied to clipboard

Published 20 hours ago • verified publisher icon mojolicious

Fixed: Mojo::DOM doesn't recognize end of comment #2029 #2030

Open poti1 opened this issue 1 year ago • 16 comments

Summary

Mojo::DOM doesn't recognize end of comment (when it should) #2030 Mojo::DOM treats "-- >" as end of comment (it shouldn't) #2029

Motivation

EXPLAIN WHY YOU BELIEVE THESE CHANGES ARE NECESSARY HERE

References

LIST RELEVANT ISSUES, PULL REQUESTS AND FORUM DISCUSSIONS HERE

poti1 avatar Jun 26 '23 20:06 poti1

That commit message tells me nothing.

kraih avatar Jun 26 '23 22:06 kraih

Since both issues were reported by @mauke, a review would be appreciated.

kraih avatar Jun 26 '23 22:06 kraih

That commit message tells me nothing.

I am not sure how to best proceed. (Should I revert and update the comment?)

poti1 avatar Jun 27 '23 04:06 poti1

I am not sure how to best proceed. (Should I revert and update the comment?)

Just change the commit message to something meaningful.

kraih avatar Jun 27 '23 09:06 kraih

You changed the title of the PR, not the commit message. I'll mark this PR as work in progress for now, so nobody reviews it yet by accident.

kraih avatar Jun 28 '23 17:06 kraih

You changed the title of the PR, not the commit message. I'll mark this PR as work in progress for now, so nobody reviews it yet by accident.

Not sure how to just change the commit message here without submitting another commit.

poti1 avatar Jun 29 '23 04:06 poti1

@poti1

git commit --amend -m <useful commit message>
git push --force <origin> <branch>

rawleyfowler avatar Jun 29 '23 14:06 rawleyfowler

I didn't know about the --ammend option. Thanks

poti1 avatar Jun 29 '23 17:06 poti1

@kraih Anything missing from my side?

poti1 avatar Jul 04 '23 11:07 poti1

Wonder if those lookahead assertions will end up being a performance issue in the future when someone finds a weird enough HTML document.

kraih avatar Aug 26 '23 12:08 kraih

I completely forgot what the fix even was now 😅

poti1 avatar Aug 26 '23 17:08 poti1

Wonder if those lookahead assertions will end up being a performance issue in the future when someone finds a weird enough HTML document.

Any suggestions?

poti1 avatar Aug 26 '23 17:08 poti1

Maybe we can use an "unrolling-the-loop" pattern 🤔

poti1 avatar Aug 26 '23 17:08 poti1

Something that's also easy to port to the JavaScript version would be neat. 😁

kraih avatar Aug 26 '23 18:08 kraih

I know Mojo::DOM. What's the JavaScript version? (I'm still not familiar with all Mojo features)

poti1 avatar Aug 26 '23 18:08 poti1

I know Mojo::DOM. What's the JavaScript version? (I'm still not familiar with all Mojo features)

https://github.com/mojolicious/dom.js

kraih avatar Aug 26 '23 19:08 kraih