FIX: Add post id to the anchor to prevent two identical anchors (#28070)

* FIX: Add post id to the anchor to prevent two identical anchors

We generate anchors for headings in posts. This works fine if there is
only one post in a topic with anchors. The problem comes when you have
two or more posts with the same heading. PrettyText generates anchors
based on the heading text using the raw context of each post, so it is
entirely possible to generate the same anchor for two posts in the same
topic, especially for topics with template replies

    Post1:
    # heading
    context
    Post2:
    # heading
    context

When both posts are on the page at the same time, the anchor will only
work for the first post, according to the [HTML specification](https://html.spec.whatwg.org/multipage/browsing-the-web.html#scroll-to-the-fragment-identifier).

> If there is an a element in the document tree whose root is document
> that has a name attribute whose value is equal to fragment, then
> return the *first* such element in tree order.

This bug is particularly serious in forums with non-Latin languages,
such as Chinese. We do not generate slugs for Chinese, which results in
the heading anchors being completely dependent on their order.

```ruby
[2] pry(main)> PrettyText.cook("# 中文")
=> "<h1><a name=\"h-1\" class=\"anchor\" href=\"#h-1\"></a>中文</h1>"
```

Therefore, the anchors in the two posts must be in exactly the same by
order, causing almost all of the anchors in the second post to be
invalid.

This commit solves this problem by adding the `post_id` to the anchor.
The new anchor generation method will add `p-{post_id}` as a prefix when
post_id is available:

```ruby
[3] pry(main)> PrettyText.cook("# 中文", post_id: 1234)
=> "<h1><a name=\"p-1234-h-1\" class=\"anchor\" href=\"#p-1234-h-1\"></a>中文</h1>"
```

This way we can ensure that each anchor name only appears once on the
same topic. Using post id also prevents the potential possibility of the
same anchor name when splitting/merging topics.
This commit is contained in:
锦心 2024-07-25 13:50:30 +08:00 committed by GitHub
parent f4d06f195d
commit 5b05cdfbd9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 33 additions and 3 deletions

View File

@ -5,6 +5,8 @@ export function setup(helper) {
helper.registerPlugin((md) => {
md.core.ruler.push("anchor", (state) => {
const postId = helper.getOptions().postId;
for (
let idx = 0, lvl = 0, headingId = 0;
idx < state.tokens.length;
@ -45,6 +47,10 @@ export function setup(helper) {
slug = `${slug || "h"}-${++headingId}`;
if (postId) {
slug = `p-${postId}-${slug}`;
}
linkOpen.attrSet("name", slug);
linkOpen.attrSet("class", "anchor");
linkOpen.attrSet("href", "#" + slug);

View File

@ -10,6 +10,7 @@ export default function buildOptions(state) {
lookupPrimaryUserGroup,
getTopicInfo,
topicId,
postId,
forceQuoteLink,
userId,
getCurrentUser,
@ -48,6 +49,7 @@ export default function buildOptions(state) {
lookupPrimaryUserGroup,
getTopicInfo,
topicId,
postId,
forceQuoteLink,
userId,
getCurrentUser,

View File

@ -666,6 +666,15 @@ eviltrout</p>
);
});
test("Heading anchors with post id", function (assert) {
assert.cookedOptions(
"# 1\n\n# one",
{ postId: 1234 },
'<h1><a name="p-1234-h-1-1" class="anchor" href="#p-1234-h-1-1"></a>1</h1>\n' +
'<h1><a name="p-1234-one-2" class="anchor" href="#p-1234-one-2"></a>one</h1>'
);
});
test("bold and italics", function (assert) {
assert.cooked(
'a "**hello**"',

View File

@ -325,6 +325,7 @@ class Post < ActiveRecord::Base
# is referencing.
options[:user_id] = self.last_editor_id
options[:omit_nofollow] = true if omit_nofollow?
options[:post_id] = self.id
if self.should_secure_uploads?
each_upload_url do |url|

View File

@ -159,6 +159,7 @@ module PrettyText
# markdown_it_rules - An array of markdown rule names which will be applied to the markdown-it engine. Currently used by plugins to customize what markdown-it rules should be
# enabled when rendering markdown.
# topic_id - Topic id for the post being cooked.
# post_id - Post id for the post being cooked.
# user_id - User id for the post being cooked.
# force_quote_link - Always create the link to the quoted topic for [quote] bbcode. Normally this only happens
# if the topic_id provided is different from the [quote topic:X].
@ -208,6 +209,7 @@ module PrettyText
JS
buffer << "__optInput.topicId = #{opts[:topic_id].to_i};\n" if opts[:topic_id]
buffer << "__optInput.postId = #{opts[:post_id].to_i};\n" if opts[:post_id]
if opts[:force_quote_link]
buffer << "__optInput.forceQuoteLink = #{opts[:force_quote_link]};\n"

View File

@ -1168,20 +1168,30 @@ RSpec.describe Post do
expect(post.cooked).to match(/noopener nofollow ugc/)
end
it "passes the last_editor_id as the markdown user_id option" do
it "passes the last_editor_id as the markdown user_id option and post_id" do
post.save
post.reload
PostAnalyzer
.any_instance
.expects(:cook)
.with(post.raw, { cook_method: Post.cook_methods[:regular], user_id: post.last_editor_id })
.with(
post.raw,
{
cook_method: Post.cook_methods[:regular],
user_id: post.last_editor_id,
post_id: post.id,
},
)
post.cook(post.raw)
user_editor = Fabricate(:user)
post.update!(last_editor_id: user_editor.id)
PostAnalyzer
.any_instance
.expects(:cook)
.with(post.raw, { cook_method: Post.cook_methods[:regular], user_id: user_editor.id })
.with(
post.raw,
{ cook_method: Post.cook_methods[:regular], user_id: user_editor.id, post_id: post.id },
)
post.cook(post.raw)
end