close
Skip to content

Fix missing header from GitLab blog page#885

Merged
fchasen merged 3 commits intomozilla:mainfrom
revolter:fix/gitlab-blog
Jun 27, 2024
Merged

Fix missing header from GitLab blog page#885
fchasen merged 3 commits intomozilla:mainfrom
revolter:fix/gitlab-blog

Conversation

@revolter
Copy link
Copy Markdown
Contributor

No description provided.

@fchasen
Copy link
Copy Markdown

fchasen commented Jun 27, 2024

Thanks for submitting the test case and fix, the simpler fix for this seem to be to just remove tool from the regex as that doesn't appear to break any other test cases.

@revolter
Copy link
Copy Markdown
Contributor Author

Wasn't it added there with a reason? Maybe there is a test case missing for that part of the regex? 🤔

@revolter
Copy link
Copy Markdown
Contributor Author

Oh, it's there since the initial commit.

So, should I remove tool completely?

@fchasen
Copy link
Copy Markdown

fchasen commented Jun 27, 2024

Yep, seems to be fine to remove completely.

Comment thread Readability.js Outdated
@revolter revolter changed the title Fixed missing header from GitLab blog page Fix missing header from GitLab blog page Jun 27, 2024
Co-authored-by: Fred Chasen <fchasen@gmail.com>
@fchasen fchasen merged commit 2d814cc into mozilla:main Jun 27, 2024
@fchasen
Copy link
Copy Markdown

fchasen commented Jun 27, 2024

Thanks again for the text case that has most of the text of the header in the id, that pattern doesn't work great with regex matching (for instance if the header was about Meta). We may look into this more soon depending on how common that is.

@revolter
Copy link
Copy Markdown
Contributor Author

Good job with the contribution instructions 👏🏻 Was thinking about creating an issue at first 😅

@revolter revolter deleted the fix/gitlab-blog branch June 28, 2024 04:47
mislav added a commit to mislav/go-readability that referenced this pull request Jun 19, 2025
This ports "Fix missing header from GitLab blog page" (mozilla/readability#885)
mislav added a commit to mislav/go-readability that referenced this pull request Jun 23, 2025
This ports "Fix missing header from GitLab blog page" (mozilla/readability#885)
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

Successfully merging this pull request may close these issues.

2 participants