Page MenuHomePhabricator

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

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

  1. On hover:

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

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

ProjectBranchLines +/-Subject
mediawiki/extensions/TwoColConflictmaster+1 -0
mediawiki/extensions/TwoColConflictmaster+22 -38
mediawiki/extensions/TwoColConflictmaster+5 -5
mediawiki/extensions/TwoColConflictmaster+8 -9
mediawiki/extensions/TwoColConflictmaster+1 -1
mediawiki/extensions/TwoColConflictmaster+6 -4
mediawiki/extensions/TwoColConflictmaster+2 -2
mediawiki/extensions/TwoColConflictmaster+7 -8
mediawiki/extensions/TwoColConflictmaster+1 -1
mediawiki/extensions/TwoColConflictmaster+2 -2
mediawiki/extensions/TwoColConflictmaster+54 -20
mediawiki/extensions/TwoColConflictmaster+10 -11
mediawiki/extensions/TwoColConflictmaster+72 -0
mediawiki/extensions/TwoColConflictmaster+2 -2
mediawiki/extensions/TwoColConflictmaster+2 -18
mediawiki/extensions/TwoColConflictmaster+13 -4
mediawiki/extensions/TwoColConflictmaster+340 -27
Show related patches Customize query in gerrit

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 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

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

WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-06-10 board.

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

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

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.

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.

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 :-).

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 added a subscriber: awight.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:

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:

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.

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.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

@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.

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

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?

@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,

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

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

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.

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

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

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

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.

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

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

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

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

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

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

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

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

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

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

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

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

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

Reason:
Obsolete via Ib089fbf.

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

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

Waiting on final style tweaks.

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

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.

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.

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

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

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

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?

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

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.

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

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