Page MenuHomePhabricator
Create Task
Maniphest T297034

Reply added inside wrapper node for "The Signpost" delivery
Closed, ResolvedPublic
Actions

Assigned To
matmarex
Authored By
matmarex
Dec 4 2021, 1:29 AM
Tags
Referenced Files
F34951333: Screenshot 2022-02-14 at 14.55.30.png
Feb 14 2022, 1:57 PM
F34951328: Screenshot 2022-02-14 at 14.52.14.png
Feb 14 2022, 1:57 PM
F34951331: Screenshot 2022-02-14 at 14.53.58.png
Feb 14 2022, 1:57 PM
F34926095: image.png
Jan 21 2022, 8:33 PM
F34926093: image.png
Jan 21 2022, 8:33 PM
F34926097: image.png
Jan 21 2022, 8:33 PM
F34825892: image.png
Dec 4 2021, 1:31 AM
F34825895: image.png
Dec 4 2021, 1:31 AM
View All 9 Files
Subscribers
Aklapper
Dylsss
EAkinloose
matmarex
Ryasmeen

Description

In these edits, the replies were added inside wrapper <div> node for "The Signpost" delivery, instead of after it:

I'm pretty sure that this worked correctly after T250126, it might be a regression.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
One more tweak for comparing comment ranges to transclusion/DOM rangesmediawiki/extensions/DiscussionToolsmaster+46 -28
Ignore some invisible nodes when looking for comment framesmediawiki/extensions/DiscussionToolsmaster+69 -156
Test cases for adding replies outside of framesmediawiki/extensions/DiscussionToolsmaster+964 -0
Customize query in gerrit

Related Objects

Event Timeline

matmarex created this task.Dec 4 2021, 1:29 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 4 2021, 1:29 AM
matmarex added a comment.Dec 4 2021, 1:31 AM

The issue is also apparent when using the reply tool before saving (note how the font from the message is applied to our interface):

image.png (2×3 px, 479 KB)

And in a few other messages on https://en.wikipedia.org/wiki/User_talk:Dylsss:

image.png (2×3 px, 403 KB)

image.png (2×3 px, 426 KB)

matmarex added a subscriber: Dylsss.Dec 4 2021, 1:37 AM

CC @Dylsss, in case you find this interesting, since you were testing this in that edit I found, and since you're active on Phabricator. I found the edits using https://dtcheck.toolforge.org/, which is a tool we built for tracking down dirty diffs in edits made with the reply tool.

gerritbot added a comment.Dec 4 2021, 2:28 AM

Change 743525 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] [WIP] Ignore some invisible nodes when looking for comment frames

https://gerrit.wikimedia.org/r/743525

gerritbot added a project: Patch-For-Review.Dec 4 2021, 2:28 AM
matmarex claimed this task.Dec 4 2021, 2:29 AM
matmarex added a project: Editing-team (Kanban Board).
matmarex moved this task from Inbox to Doing on the Editing-team (Kanban Board) board.
matmarex moved this task from Backlog to Triaged on the DiscussionTools board.
gerritbot added a comment.Dec 16 2021, 10:58 AM

Change 747812 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] Test cases for adding replies outside of frames

https://gerrit.wikimedia.org/r/747812

matmarex moved this task from Doing to Code Review on the Editing-team (Kanban Board) board.Dec 21 2021, 3:43 PM
gerritbot added a comment.Jan 11 2022, 5:20 PM

Change 747812 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Test cases for adding replies outside of frames

https://gerrit.wikimedia.org/r/747812

gerritbot added a comment.Jan 11 2022, 5:20 PM

Change 743525 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Ignore some invisible nodes when looking for comment frames

https://gerrit.wikimedia.org/r/743525

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.18; 2022-01-17).Jan 11 2022, 6:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jan 11 2022, 6:10 PM
matmarex added a project: Editing QA.Jan 11 2022, 6:54 PM
matmarex moved this task from Code Review to QA on the Editing-team (Kanban Board) board.

QA note: This can be tested without saving the comments, see T297034#7547546.

matmarex moved this task from QA to Blocked / Needs More Work on the Editing-team (Kanban Board) board.Jan 21 2022, 8:33 PM
matmarex removed a project: Editing QA.
matmarex added a subscriber: Ryasmeen.

@Ryasmeen noticed that it's only partially fixed:

In T297034#7547546, @matmarex wrote:

The issue is also apparent when using the reply tool before saving (note how the font from the message is applied to our interface):
…
And in a few other messages on https://en.wikipedia.org/wiki/User_talk:Dylsss:

BeforeAfterStatus
image.png (2×3 px, 479 KB)
image.png (2×3 px, 480 KB)
fixed
image.png (2×3 px, 403 KB)
image.png (2×3 px, 428 KB)
NOT FIXED
image.png (2×3 px, 426 KB)
image.png (2×3 px, 411 KB)
fixed
gerritbot added a comment.Jan 24 2022, 5:25 PM

Change 756637 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/DiscussionTools@master] One more tweak for comparing comment ranges to transclusion/DOM ranges

https://gerrit.wikimedia.org/r/756637

gerritbot added a project: Patch-For-Review.Jan 24 2022, 5:25 PM
matmarex moved this task from Blocked / Needs More Work to Code Review on the Editing-team (Kanban Board) board.Jan 24 2022, 5:37 PM
gerritbot added a comment.Jan 24 2022, 11:37 PM

Change 756637 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] One more tweak for comparing comment ranges to transclusion/DOM ranges

https://gerrit.wikimedia.org/r/756637

ReleaseTaggerBot edited projects, added MW-1.38-notes (1.38.0-wmf.19; 2022-01-24); removed MW-1.38-notes (1.38.0-wmf.18; 2022-01-17).Jan 25 2022, 12:00 AM
Maintenance_bot removed a project: Patch-For-Review.Jan 25 2022, 12:11 AM
matmarex moved this task from Code Review to QA on the Editing-team (Kanban Board) board.Jan 25 2022, 10:38 AM
matmarex added a project: Editing QA.
EAkinloose subscribed.Feb 14 2022, 1:57 PM
In T297034#7641334, @matmarex wrote:

@Ryasmeen noticed that it's only partially fixed:

In T297034#7547546, @matmarex wrote:

The issue is also apparent when using the reply tool before saving (note how the font from the message is applied to our interface):
…
And in a few other messages on https://en.wikipedia.org/wiki/User_talk:Dylsss:

BeforeAfterStatus
image.png (2×3 px, 479 KB)
image.png (2×3 px, 480 KB)
fixed
image.png (2×3 px, 403 KB)
image.png (2×3 px, 428 KB)
NOT FIXED
image.png (2×3 px, 426 KB)
image.png (2×3 px, 411 KB)
fixed

All good now. Following the same order as above, the fixed versions look like:

Screenshot 2022-02-14 at 14.55.30.png (1×2 px, 369 KB)

Screenshot 2022-02-14 at 14.52.14.png (956×2 px, 218 KB)

Screenshot 2022-02-14 at 14.53.58.png (1×2 px, 317 KB)

@matmarex , let me know if this doesn't align with the expected result.

EAkinloose edited projects, added Verified; removed Editing QA.Feb 14 2022, 1:57 PM
EAkinloose moved this task from QA to Ready for Sign Off on the Editing-team (Kanban Board) board.
ppelberg closed this task as Resolved.Feb 18 2022, 3:12 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptFeb 18 2022, 3:12 AM
matmarex mentioned this in T273692: Reply inserted before closing wrapper tags at end of line (round 2).Aug 5 2022, 11:28 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits