Page MenuHomePhabricator
Create Task
Maniphest T254440

(No-JS) View "your revision" source code in new tab
Closed, ResolvedPublic8 Estimated Story Points
Actions

Assigned To
WMDE-Fisch
Authored By
ECohen_WMDE
Jun 4 2020, 8:44 AM
Tags
Referenced Files
F31903820: image.png
Jun 24 2020, 1:35 PM
F31874896: image.png
Jun 22 2020, 1:50 PM
F31874885: image.png
Jun 22 2020, 1:41 PM
F31871633: image.png
Jun 19 2020, 6:09 PM
F31871620: image.png
Jun 19 2020, 5:47 PM
F31871612: image.png
Jun 19 2020, 5:47 PM
F31868841: Screenshot_2020-06-17 Copy your edit - EnLocalWiki.png
Jun 17 2020, 5:42 AM
F31853289: No-JS New Tab - Revision Text.png
Jun 4 2020, 8:51 AM
View All 11 Files
Subscribers
Aklapper
awight
ECohen_WMDE
Lena_WMDE
WMDE-Fisch

Description

Acceptance Criteria

  • On hover, "(View full text)" link shows browser tooltip.
  • On click, "(View full text)" link opens a new tab with source code of user's full 'your revision' text, without including any edits made in the 2ColConf interface
  • When in the new tab, users can click in to the source code textbox and use CTRL-A to select all and CTRL-C to copy contents to clipboard

Mocks

  1. Add link/button to title

No JS - Copy.png (778×1 px, 201 KB)

  1. On hover:

No JS Copy - Hover.png (778×1 px, 210 KB)

  1. New tab (use same textbox as source code for protected pages, use same spacing above/below for "Your revision" title as 2Col interface):

No-JS New Tab - Revision Text.png (902×1 px, 187 KB)

Wording

  1. "(View full text)" added to the "Your revision" title in the no-JS 2ColConf interface.
  2. Hover text: "View full wikitext of your revision (edits made below are not included). Opens in a new tab."
  3. New tab, text for warning notice: "While you were editing, another user edited the same page and published before you. A view-only copy of your revision is preserved below. Any changes made in the edit conflict interface are not reflected. To merge changes and resolve the issue using the edit conflict interface, return to the previous tab." Above text box: "You can view and copy the source of your revision:" Below text box: "Close current window to return to the edit conflict interface."

Details

SubjectRepoBranchLines +/-
Improve test coverage for "copy text" special page linkmediawiki/extensions/TwoColConflictmaster+10 -10
Add missing blue color to "copy" special pagemediawiki/extensions/TwoColConflictmaster+1 -0
Merge the 2 JS/no-JS "copy" links into a single onemediawiki/extensions/TwoColConflictmaster+22 -38
Always cache unsaved text, not only in no-JS modemediawiki/extensions/TwoColConflictmaster+5 -5
Don't hide help message on the "copy" special page in JS modemediawiki/extensions/TwoColConflictmaster+8 -9
Add missing whitespace before "copy" linkmediawiki/extensions/TwoColConflictmaster+1 -1
Add missing margin after message box on "copy" special pagemediawiki/extensions/TwoColConflictmaster+6 -4
Simplify title parammediawiki/extensions/TwoColConflictmaster+2 -2
Use subpage for passing titlemediawiki/extensions/TwoColConflictmaster+7 -8
Downgrade conflict message to "notice"mediawiki/extensions/TwoColConflictmaster+1 -1
Revert "Simple parameter name for special page"mediawiki/extensions/TwoColConflictmaster+2 -2
Fix possible cache key conflict in SubmittedTextCachemediawiki/extensions/TwoColConflictmaster+54 -20
Stashed page contents can be restoredmediawiki/extensions/TwoColConflictmaster+10 -11
Tests for SubmittedTextCachemediawiki/extensions/TwoColConflictmaster+72 -0
Simple parameter name for special pagemediawiki/extensions/TwoColConflictmaster+2 -2
Merge stylesmediawiki/extensions/TwoColConflictmaster+2 -18
Extend header tests to include copy linksmediawiki/extensions/TwoColConflictmaster+13 -4
Provide a page to copy the submitted textmediawiki/extensions/TwoColConflictmaster+340 -27
Show related patches Customize query in gerrit

Related Objects

Event Timeline

ECohen_WMDE created this task.Jun 4 2020, 8:44 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 4 2020, 8:44 AM
ECohen_WMDE moved this task from Backlog to Tickets ready for pickup on the Two-Column-Edit-Conflict-Merge board.Jun 4 2020, 8:44 AM
ECohen_WMDE updated the task description. (Show Details)Jun 4 2020, 8:51 AM
Lena_WMDE set the point value for this task to 8.Jun 4 2020, 9:13 AM
WMDE-Fisch subscribed.Jun 5 2020, 11:28 AM

Just for the implementation idea here. I gave it some thought while working on the JS version.

We originally wanted the link to be a submit button that sits in it's own form with target="_blank" so we can open a new tab. Then we could have the text as a hidden input transmitted via post to the SpecialPage that shows the textarea. - This will not work, because DOM-wise the 2nd form sits inside of the edit page form and unfortunately this prevents us from implementing it like this.

That's the new plan:

  • Store the users text in cache when the page is generated. Including the users session id and maybe even the user id.
  • Use a link with target ="_blank" and a key to the cache entry as parameter.
  • The SpecialPage takes the key validates the cached content with the user and session and provides the textarea with the content.

Caching can be done like https://gerrit.wikimedia.org/g/mediawiki/extensions/FileImporter/+/06bc144cd47e79b514698e8acf85481a804a86af/src/Services/SuccessCache.php

Lena_WMDE added a project: WMDE-QWERTY-Sprint-2020-06-10.Jun 10 2020, 12:07 PM
WMDE-Fisch claimed this task.Jun 11 2020, 1:24 PM
WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-06-10 board.
gerritbot added a comment.Jun 12 2020, 1:09 PM

Change 605216 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] [WIP] Provide a page to copy the submitted text

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

gerritbot added a project: Patch-For-Review.Jun 12 2020, 1:09 PM
WMDE-Fisch added a comment.Jun 17 2020, 5:42 AM

@ecohen @Lena_WMDE One question to the page that shows the text to copy: What should be the heading of that page?

Screenshot_2020-06-17 Copy your edit - EnLocalWiki.png (610×1 px, 63 KB)

It could be any text that fits and it could include the title of the original page the user wanted to edit. Not super urgent but would be nice to the end of the week.

Lena_WMDE added a comment.Jun 17 2020, 7:42 AM

thanks @WMDE-Fisch! Elisha and I thought about this and what we landed on was that it should be the same title as the Edit Conflict you're coming from (which is what we used in screenshot 3 in the ticket) so that you still have the context of what you're looking at/why you're looking at it.

WMDE-Fisch added a comment.Jun 17 2020, 7:49 AM
In T254440#6230683, @Lena_WMDE wrote:

thanks @WMDE-Fisch! Elisha and I thought about this and what we landed on was that it should be the same title as the Edit Conflict you're coming from (which is what we used in screenshot 3 in the ticket) so that you still have the context of what you're looking at/why you're looking at it.

Ok, makes sense, I was not sure if this was intended :-).

gerritbot added a comment.Jun 18 2020, 11:02 AM

Change 606409 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/TwoColConflict@master] Extend header tests to include copy links

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

awight subscribed.Jun 19 2020, 5:47 PM

There seems to be an edge case for the special page: once my session timed out and the browser refreshed itself, I got this blank page:

image.png (321×1 px, 42 KB)

We should explicitly tell the user what happened--hopefully their original conflict is alive in another tab. Just for comparison, here's how the same edge case is handled by the edit page:

image.png (491×1 px, 67 KB)

awight added a comment.Jun 19 2020, 6:09 PM
In T254440#6230683, @Lena_WMDE wrote:

thanks @WMDE-Fisch! Elisha and I thought about this and what we landed on was that it should be the same title as the Edit Conflict you're coming from (which is what we used in screenshot 3 in the ticket) so that you still have the context of what you're looking at/why you're looking at it.

image.png (350×1 px, 48 KB)

Reusing the title and header is a really neat way to hint about why this tab is open. I support this approach, but there's something about the error box which feels wrong. Maybe the error box should be downgraded to a warning or even a plain notice box? It's purely informational after all, and although it refers to the blocking problem in another tab, it isn't blocking the copy page.

Meanwhile, the feedback notice is just downright distracting :-) but I understand if we want to keep it front and center while users adapt to the new interface.

gerritbot added a comment.Jun 19 2020, 7:12 PM

Change 605216 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Provide a page to copy the submitted text

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

gerritbot added a comment.Jun 19 2020, 7:35 PM

Change 606756 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Tests for SubmittedTextCache

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

gerritbot added a comment.Jun 19 2020, 7:35 PM

Change 606757 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Stashed page contents can be restored

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

gerritbot added a comment.Jun 19 2020, 7:41 PM

Change 606759 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Simple parameter name for special page

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

gerritbot added a comment.Jun 19 2020, 7:57 PM

Change 606761 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Merge styles

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.38; 2020-06-23).Jun 19 2020, 8:00 PM
gerritbot added a comment.Jun 19 2020, 8:18 PM

Change 606409 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Extend header tests to include copy links

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

gerritbot added a comment.Jun 20 2020, 10:32 AM

Change 606761 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Merge styles

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

gerritbot added a comment.Jun 20 2020, 10:32 AM

Change 606759 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Simple parameter name for special page

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

gerritbot added a comment.Jun 20 2020, 10:36 AM

Change 606756 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Tests for SubmittedTextCache

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

gerritbot added a comment.Jun 21 2020, 7:15 AM

Change 606757 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Stashed page contents can be restored

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

Maintenance_bot removed a project: Patch-For-Review.Jun 21 2020, 8:10 AM
gerritbot added a comment.Jun 22 2020, 8:09 AM

Change 606954 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Fix possible cache key conflict in SubmittedTextCache

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

gerritbot added a project: Patch-For-Review.Jun 22 2020, 8:09 AM
gerritbot added a comment.Jun 22 2020, 9:10 AM

Change 606954 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Fix possible cache key conflict in SubmittedTextCache

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

Maintenance_bot removed a project: Patch-For-Review.Jun 22 2020, 10:10 AM
ECohen_WMDE added a comment.Jun 22 2020, 10:40 AM

@awight Thanks for bringing this up! That's a very good point that the action they just took (clicking on copy) was 100% successful and it could be very confusing to see this warning message. After discussion with Lena, we actually decided to go the opposite direction and make it the gray, neutral message since nothing went wrong and it's purely informational.

The feedback message should also stay. It's only temporary, so eventually it'll get taken out and it will only be the one gray message at the top, cleaning things up.

WMDE-Fisch moved this task from Doing to Demo on the WMDE-QWERTY-Sprint-2020-06-10 board.Jun 22 2020, 12:38 PM
awight moved this task from Demo to Doing on the WMDE-QWERTY-Sprint-2020-06-10 board.Jun 22 2020, 1:41 PM

Something's wrong--I tested this as an anonymous user and I get no message boxes, which is very confusing:

image.png (449×710 px, 30 KB)

awight added a comment.Jun 22 2020, 1:44 PM

It's a very strange experience--I have URL http://devwiki.local/w/index.php?title=Special:ProvideSubmittedText&title=User%3AAdmin%2Fsandbox but my tabs look like I'm on an ordinary user page. Maybe the tabs are a second bug?

Lena_WMDE moved this task from Tickets ready for pickup to Tickets in sprint on the Two-Column-Edit-Conflict-Merge board.Jun 22 2020, 1:45 PM
awight added a comment.Jun 22 2020, 1:50 PM

@ecohen I think the tabs issue above is a design question. In the mockups and in the current impementation, tab actions look like we're on an edit page. For comparison, here's what tabs look like on most other special pages,

image.png (247×763 px, 24 KB)

I don't have a strong opinion, just wanted to flag something odd.

ECohen_WMDE added a comment.Jun 22 2020, 2:26 PM

Ah good catch - ya that was an accident as I didn't realize that special pages looked like that. Since it is not editable, then I think matching the existing standard makes sense (single "special page" tab). As long as the title matches as discussed above, then I think it should be clear.

What the bug though where you get no message boxes? That seems like a bigger issue

awight added a comment.Jun 22 2020, 3:18 PM

I see the issue now, it's due to my patch to "simplify" the parameter to use "title". If you access the special page like .../wiki/Special:ProvideSubmittedText?title=... then you're okay, but if you access like .../w/index.php?title=Special:ProvideSubmittedText&title=... then you're in trouble. I'll change the parameter name again.

gerritbot added a comment.Jun 22 2020, 3:21 PM

Change 607058 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Revert "Simple parameter name for special page"

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

gerritbot added a project: Patch-For-Review.Jun 22 2020, 3:21 PM
gerritbot added a comment.Jun 22 2020, 3:26 PM

Change 607061 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Simplify title param

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

gerritbot added a comment.Jun 22 2020, 3:29 PM

Change 607062 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Downgrade conflict message to "notice"

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

awight moved this task from Doing to Review on the WMDE-QWERTY-Sprint-2020-06-10 board.Jun 22 2020, 3:30 PM

Two minor fixups, including the error -> notice downgrade, are now pending review. We might miss the branch point for these, but IMO they're lower-priority and it's fine to wait a week.

gerritbot added a comment.Jun 22 2020, 3:43 PM

Change 607058 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Revert "Simple parameter name for special page"

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

gerritbot added a comment.Jun 22 2020, 5:09 PM

Change 607062 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Downgrade conflict message to "notice"

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

gerritbot added a comment.Jun 23 2020, 7:22 AM

Change 607224 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/TwoColConflict@master] Use subpage for passing title

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

WMDE-Fisch moved this task from Review to Demo on the WMDE-QWERTY-Sprint-2020-06-10 board.Jun 23 2020, 8:58 AM
gerritbot added a comment.Jun 23 2020, 9:06 AM

Change 607243 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Add missing blue color to "copy" special page

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

gerritbot added a comment.Jun 23 2020, 9:14 AM

Change 607246 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Don't hide help message on the "copy" special page in JS mode

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

gerritbot added a comment.Jun 23 2020, 9:22 AM

Change 607224 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Use subpage for passing title

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

gerritbot added a comment.Jun 23 2020, 9:24 AM

Change 607247 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Always cache unsaved text, not only in no-JS mode

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

gerritbot added a comment.Jun 23 2020, 9:39 AM

Change 607252 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Add missing whitespace before "copy" link

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

gerritbot added a comment.Jun 23 2020, 9:53 AM

Change 607255 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Merge the 2 JS/no-JS "copy" links into a single one

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

gerritbot added a comment.Jun 23 2020, 9:57 AM

Change 607061 abandoned by Thiemo Kreuz (WMDE):
Simplify title param

Reason:
Obsolete via Ib089fbf.

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

ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.39; 2020-06-30); removed MW-1.35-notes (1.35.0-wmf.38; 2020-06-23).Jun 23 2020, 10:00 AM
gerritbot added a comment.Jun 23 2020, 10:12 AM

Change 607259 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/TwoColConflict@master] Add missing margin after message box on "copy" special page

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

awight added a comment.Jun 23 2020, 11:44 AM

Waiting on final style tweaks.

gerritbot added a comment.Jun 24 2020, 6:04 AM

Change 607259 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add missing margin after message box on "copy" special page

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

gerritbot added a comment.Jun 24 2020, 6:04 AM

Change 607252 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add missing whitespace before "copy" link

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

WMDE-Fisch closed this task as Resolved.Jun 24 2020, 6:11 AM
WMDE-Fisch moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2020-06-10 board.
In T254440#6251000, @gerritbot wrote:

Change 607259 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add missing margin after message box on "copy" special page

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

Margin related style tweaks are merged.

gerritbot added a comment.Jun 24 2020, 6:13 AM

Change 607246 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Don't hide help message on the "copy" special page in JS mode

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

gerritbot added a comment.Jun 24 2020, 1:15 PM

Change 607247 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Always cache unsaved text, not only in no-JS mode

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

awight added a comment.Jun 24 2020, 1:35 PM

@ecohen We noticed a small glitch, that the mockup is lacking background colors for each column heading. For example,

image.png (293×1 px, 40 KB)

Do you think the single-column special page for copying text should have a colored background behind "Your text", for consistency with the two-column page?

gerritbot added a comment.Jun 24 2020, 2:22 PM

Change 607255 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Merge the 2 JS/no-JS "copy" links into a single one

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

ECohen_WMDE added a comment.Jun 24 2020, 3:15 PM

Good catch. Yes, that's a good idea for the "Your revision" title on the new tab page to also have the blue background highlight.

gerritbot added a comment.Jun 25 2020, 6:53 AM

Change 607243 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Add missing blue color to "copy" special page

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

Maintenance_bot removed a project: Patch-For-Review.Jun 25 2020, 7:10 AM
thiemowmde mentioned this in T298039: Migrate TwoColConflict usage of objectcache away from the table.Dec 20 2021, 7:48 PM
gerritbot added a comment.Jan 3 2024, 9:32 AM

Change 987404 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/TwoColConflict@master] Improve test coverage for "copy text" special page link

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

gerritbot added a project: Patch-For-Review.Jan 3 2024, 9:32 AM
gerritbot added a comment.Jan 15 2024, 10:18 AM

Change 987404 merged by jenkins-bot:

[mediawiki/extensions/TwoColConflict@master] Improve test coverage for "copy text" special page link

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

thiemowmde mentioned this in rESCC0be0c21e98d1: Improve test coverage for "copy text" special page link.Jan 15 2024, 10:19 AM
Maintenance_bot removed a project: Patch-For-Review.Jan 15 2024, 10:31 AM
ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.14; 2024-01-16).Jan 15 2024, 11:00 AM
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