Page MenuHomePhabricator
Differential D18465

Prevent Differential changeset HTML anchors from colliding with comment anchors
ClosedPublic
Actions

Authored by epriestley on Aug 24 2017, 8:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 24, 10:20 PM
Unknown Object (File)
Fri, Jan 24, 10:20 PM
Unknown Object (File)
Fri, Jan 24, 10:20 PM
Unknown Object (File)
Dec 20 2024, 11:26 PM
Unknown Object (File)
Dec 20 2024, 6:50 PM
Unknown Object (File)
Dec 12 2024, 2:39 AM
Unknown Object (File)
Dec 5 2024, 9:22 PM
Unknown Object (File)
Nov 27 2024, 5:57 AM
View All Files
Subscribers
cspeckmim

Details

Reviewers
chad
Maniphest Tasks
T12970: Differential HTML document anchors for changesets and comments may collide
Commits
rPba1925b15515: Prevent Differential changeset HTML anchors from colliding with comment anchors
Summary

Fixes T12970. This is easier than I expected, and appears to occur in only one place.

This prevents a change from ever generating with an anchor like #12345678, which is ambiguous because it may be a comment anchor.

Test Plan

Viewed a revision, saw new change-xxxyyyzzz anchors, clicked one, got jumped to the right place.

Diff Detail

Repository
rP Phabricator
Branch
hash1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 18147
Build 24392: Run Core Tests
Build 24391: arc lint + arc unit

Event Timeline

epriestley created this revision.Aug 24 2017, 8:40 PM
Harbormaster completed remote builds in B18147: Diff 44360.Aug 24 2017, 8:41 PM
chad accepted this revision.Aug 24 2017, 8:52 PM
This revision is now accepted and ready to land.Aug 24 2017, 8:52 PM
Closed by commit rPba1925b15515: Prevent Differential changeset HTML anchors from colliding with comment anchors (authored by epriestley). · Explain WhyAug 24 2017, 10:25 PM
This revision was automatically updated to reflect the committed changes.
cspeckmim added a subscriber: cspeckmim.Aug 25 2017, 12:05 AM

Would this break anyone's bookmarks to file locations in a diff~

epriestley added a comment.Aug 25 2017, 12:46 AM

Yes, but surely no one has those?

cspeckmim added a comment.Aug 25 2017, 12:53 AM

I don't know anyone that has those

tbh it sounds like something only weirdo internet experts would do

epriestley mentioned this in D18609: Fix an outdated HTML anchor link in Diffusion table of contents.Sep 15 2017, 12:34 PM
epriestley mentioned this in rP5ae3af6691ca: Fix an outdated HTML anchor link in Diffusion table of contents.Sep 15 2017, 3:37 PM
cspeckmim mentioned this in T12970: Differential HTML document anchors for changesets and comments may collide.Jan 22 2018, 3:01 PM
cspeckmim mentioned this in T13045: Differential file anchors may contain characters which prevent remarkup linking properly.Jan 22 2018, 3:51 PM

Revision Contents
Changeset List

PathSize
src/
applications/
differential/
storage/
2 lines

Diff 44360

View Options

src/applications/differential/storage/DifferentialChangeset.php

Loading...
Phabricator · Built by Phacility