Skip to content

Fix HTML in skeletons (ampersands must be escaped)#109360

Merged
auto-submit[bot] merged 1 commit intoflutter:masterfrom
Hixie:skeleton
Aug 11, 2022
Merged

Fix HTML in skeletons (ampersands must be escaped)#109360
auto-submit[bot] merged 1 commit intoflutter:masterfrom
Hixie:skeleton

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented Aug 11, 2022

Technically we should also be escaping the IDs and channel names and such but it's safe if we can guarantee they are always alphanumeric.

Technically we should also be escaping the IDs and channel names and such but it's safe if we can guarantee they are always alphanumeric.
@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Aug 11, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@Hixie
Copy link
Contributor Author

Hixie commented Aug 11, 2022

Not really sure how to test this...

@Hixie
Copy link
Contributor Author

Hixie commented Aug 11, 2022

cc @gspencergoog

@gspencergoog
Copy link
Contributor

Well, you could maybe check for a valid link in sanityCheckDocs in dartdoc.dart. I'm not sure it's really testing more than just that the skeleton gets filled in, but I suppose it's something.

@gspencergoog gspencergoog self-requested a review August 11, 2022 01:25
@Hixie
Copy link
Contributor Author

Hixie commented Aug 11, 2022

I looked into what a sensible test would be but short of actually validating the HTML I don't think there's much point. I filed #109362 for that. I considered just duplicating the template into the sanityCheckDocs test but it already checks for the bulk of what matters, it just doesn't quite hit any of the ampersands, looking at the HTML spec in more detail, technically I don't think they actually have to be escaped most of the time anyway, it's just good practice.

@Hixie
Copy link
Contributor Author

Hixie commented Aug 11, 2022

test-exempt: see above

@Hixie
Copy link
Contributor Author

Hixie commented Aug 11, 2022

(Assuming you're ok with the fix, feel free to add the autosubmit label when reviewing.)

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 11, 2022
@auto-submit auto-submit bot merged commit de938e5 into flutter:master Aug 11, 2022
@gspencergoog
Copy link
Contributor

I agree that there's not a lot to test here, really.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants