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

Disallow breaking on rb tags when layout aware scanning is enabled #1006

Merged
merged 2 commits into from
May 29, 2024

Conversation

Kuuuube
Copy link
Member

@Kuuuube Kuuuube commented May 29, 2024

rb tags should be ignored by layout aware scanning. This bug only appears on firefox presumably due to chromium not properly supporting rb tags.

Fixes #888

@Kuuuube Kuuuube added kind/bug The issue or PR is regarding a bug browser/firefox The issue is Firefox-only labels May 29, 2024
@Kuuuube Kuuuube requested a review from a team as a code owner May 29, 2024 16:10
@jamesmaa
Copy link
Collaborator

Are there cases in which the rb content should not be ignored?

@Kuuuube
Copy link
Member Author

Kuuuube commented May 29, 2024

This change does not make it ignore rb content. This makes it so it doesn't interpret rb tags as a line break and it instead keeps scanning further.

Here is an example of an rb tag being used:

<ruby><rb></rb><rt></rt><rb></rb><rt></rt></ruby>しい

Yomitan on firefox with layout aware scanning will only allow scanning 美, 味, or しい. not 美味しい. This change allows it to instead scan 美味しい.

For clarity, rb tags are deprecated and should not be used but they are still present in some cases. The correct way to represent the above word with furigana would be:

<ruby><rt></rt><rt></rt></ruby>しい

@jamesmaa
Copy link
Collaborator

Dumb, unrelated question but how is yomitan avoiding scanning 美お味いしい?

jamesmaa
jamesmaa previously approved these changes May 29, 2024
@Kuuuube
Copy link
Member Author

Kuuuube commented May 29, 2024

Dumb, unrelated question but how is yomitan avoiding scanning 美お味いしい?

Not totally sure how it works but we have this which seems to be doing some sort of filtering:

https://github.com/themoeway/yomitan/blob/dd1f195cd721948732abbbb2428999393844487e/ext/js/dom/dom-text-scanner.js#L393-L410

If you try to scan an rt tag Yomitan just wont let you.

@Kuuuube Kuuuube dismissed stale reviews from StefanVukovic99 and jamesmaa via 5ac9504 May 29, 2024 20:29
@Kuuuube
Copy link
Member Author

Kuuuube commented May 29, 2024

Apparently not marking the tag as enterable slightly breaks the {sentence} handlebar even though scanning works fine. Fixed that. This should be totally good to go now.

@jamesmaa jamesmaa added this pull request to the merge queue May 29, 2024
Merged via the queue into yomidevs:master with commit 7fc2f41 May 29, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser/firefox The issue is Firefox-only kind/bug The issue or PR is regarding a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yomitan cuts off sentence when <rb> tags are present if Layout-aware scanning is enabled
3 participants