Commit graph

42 commits

Author SHA1 Message Date
MusikAnimal
d3903b9c2a TextSlotDiffRenderer: show tooltip instead of help icon on inline switch
Also add to the switch itself, not just the label

Bug: T346429
Change-Id: Ia7319a63878e029765c08a44f890fe96205c35da
2023-10-17 12:31:49 -04:00
hmonroy
aae15dca22 diff: add help message to inline switch
Add help message that explains what the inline toggle does when you click
it.

Remove 'diff-table-format-label` text since it is not being used.

Bug: T346429
Change-Id: Idc3aefffd03d491a573b27c7e18b337f68bbc9cf
2023-10-13 12:09:40 -07:00
Bartosz Dziewoński
d45f68b1a9 Infuse the inline diff toggle's layout to fix accessibility
Infusing just the toggle widget, without the layout wrapping it,
causes them to get disconnected, which breaks some accessibility
features, including lack of "aria-labelledby" and no support for
activating the toggle by clicking on its label.

Bug: T346132
Change-Id: I227f47d4e6c25e19b4c02a16c287fc481efc2bf4
2023-09-16 01:14:30 +02:00
TheresNoTime
b20ea3a54f DiffToggleSwitch: remove temporary inline switch flag
Remove `$wgShowDiffToggleSwitch`, ensure the inline switcher
is always shown.

Bug: T341630
Change-Id: I37d1abe0d9773654df24dd8316cd1b351e3b91a4
2023-09-06 20:13:00 +00:00
hmonroy
13ffa17b71 DiffToggleSwitch: use PHP ToggleSwitchWidget
Add logic that renders the new OOUI/ToggleSwitchWidget (just added to
OOUI) in PHP for the inline switch. This will provide no-JS support, and will get rid of the flash when loading the page for JS users.

Bug: T341955
Change-Id: Ifc8104937cd23e9b85208b7f283db238a0e6849d
2023-09-01 15:49:15 -07:00
Amir Sarabadani
f4e68e055f Reorg: Move Status to MediaWiki\Status\
This class is used heavily basically everywhere, moving it to Utils
wouldn't make much sense. Also with this change, we can move
StatusValue to MediaWiki\Status as well.

Bug: T321882
Depends-On: I5f89ecf27ce1471a74f31c6018806461781213c3
Change-Id: I04c1dcf5129df437589149f0f3e284974d7c98fa
2023-08-25 15:44:17 +02:00
Tim Starling
8d69f99a1a Don't show the inline toggle selector on Special:MobileDiff
Have Article opt in to inline toggle switch display, so that random
callers of DifferenceEngine::showDiffPage() do not receive a
non-functional format selector.

Bug: T342158
Change-Id: Id4e38c2a20b5381e5f70b1244304da2895eaf8e7
2023-07-26 05:16:55 +00:00
Tim Starling
2b608a35ed Add an API-only user preference for diff type
* Add an API-only user preference for diff type
* Retain query parameter stickiness but fix it so that the diff-type
  from user preferences does not leak into the query string. So the
  no-JS control continues to work. If a no-JS user has the inline
  preference set, respect it on the initial load, but add an explicit
  diff-type=table to the query string of the table link so that the
  preference can be overridden.
* Get the diff type from the initial state of the no-JS button. Infuse
  the widget to access the active property using the OOUI public API.
* For a logged-in user, when the JS toggle is clicked, fetch the new
  diff body and update the user preference.
* For an anonymous user, when the JS toggle is clicked, update the
  prev/next links to include the correct diff-type query parameter.

Bug: T336713
Change-Id: Ie409d79ac8222dfa6ec8fd170b76be088be80b3a
2023-07-25 10:49:51 +10:00
Tim Starling
4014f56d73 Add SlotDiffRenderer::localizeDiff()
It makes sense for SlotDiffRenderer to localize the diffs that it
returns.

DifferenceEngine can't conveniently use this right now since it stores
all slot diffs concatenated together in a cache entry. It can't break
the string apart again to call the right SlotDiffRenderer on each
slot diff. So it assumes there must be a text diff in there somewhere
and calls TextDiffer over the whole thing.

EntitySchema does not have this problem and can benefit from this
method.

Bug: T342209
Change-Id: Ie06e100bb0e945de5dbe3dade14b2e6b706e078d
2023-07-20 10:09:42 +10:00
Tim Starling
2aa87cdf2c Factor out TextDiffer hierarchy from TextSlotDiffRenderer
* Follow the TODO comment in TextSlotDiffRenderer
  ::getTextDiffInternal() by moving the code out to three parallel
  implementations, namely ExternalTextDiffer, PhpTextDiffer and
  Wikidiff2TextDiffer.
* Add a container/factory class ManifoldTextDiffer to glue them
  together and collate available formats.
* Move the inline legend to Wikidiff2TextDiffer. Not the toggle since
  the ability to toggle depends on the available format, not the current
  format.
* Update the diff cache keys so that ManifoldTextDiffer can store the
  engine=>format map it used to generate the diff.
* Drop support for the second parameter to TextSlotDiffRenderer
 ::setEngine(), since nothing used it anymore.
* Provide a format batch API, since some engines are able to efficiently
  generate multiple formats. This might be used by DifferenceEngine in
  future.

Needs risky change notification for the cache key change.

Bug: T339184
Depends-On: I8a35b9b8ec1622c9a36d2496bdd24f51bc52c85f
Change-Id: I5c506e39162855aff53dd420dd8145156739059c
2023-07-19 12:38:18 +10:00
Tim Starling
5d26080d62 Fix Chinese diff segmentation
segmentForDiff() is supposed to allow character-level diffing of Chinese
text, by adding spaces and then removing them after the diff is
complete. But when I tested it for I2d0a6996b02d37a3, unsegmentForDiff()
failed to remove the spaces, since there was an <ins> tag between the space
and the Chinese character.

So instead, use formfeed characters to separate the Chinese characters,
and strip them unconditionally instead of relying on them being next to
Chinese characters.

Add test.

Change-Id: I230d8261bbda34ad313785a1f7c31d4db7bf989b
2023-07-06 18:14:30 +10:00
Tim Starling
36b92d45db Inject page language into TextSlotDiffRenderer
Fix fixme by sending the language code to SlotDiffRenderer as an option.

Note that this is the language used for word segmentation of the
content, not the UI language.

Change-Id: I2d0a6996b02d37a3f2d8fefa851244803025bb6c
2023-07-06 10:35:03 +10:00
Tim Starling
4d0604d8fa Remove hard-deprecated public properties of DifferenceEngine
And remove commented-out code also dating from 1.32 with no near-term
prospect of being implemented.

Change-Id: I526c558b2efae4b77ae38d0b5bb8127c8b31c280
2023-07-03 16:51:22 +10:00
Sam Wilson
ae46eb63d5 Switch back to oo-ui-element-hidden from mw-diff-element-hidden
Using the OOUI class name is an established pattern outside of
OOUI widgets, so it's easier to stick with it for the diff page
hiding and showing (switching to and from inline/table diffs, etc.).

Bug: T324759
Change-Id: I805b6b71d8e137eaa3e000b15455557df42af838
2023-06-26 19:14:41 +08:00
Tim Starling
cfabbc842a Show the inline legend even if $wgShowDiffToggleSwitch = false
Change-Id: I33549fedf3f80b5542d16c3fb96c96a99da0459e
2023-06-26 12:29:31 +10:00
Tim Starling
8983c9d862 diff: Move SlotDiffRenderer::getTablePrefix() parts assembly up to DifferenceEngine
getTablePrefix() is used to show the inline legend and inline switcher.
It is not yet part of a released stable interface.

Theoretically there may be multiple text slots on a page, and we don't
want multiple inline legends. There was already a fragment assembly
system, for the benefit of hook handlers, so move that up to the page
level, so that it can also deduplicate prefix fragments coming from
each slot.

Add tests.

Bug: T324759
Change-Id: I9baa5c24128c63bc318ba13e83a024843f4ab15e
2023-06-26 11:28:59 +10:00
hmonroy
360182a566 diff: Add inline format switch when Wikidiff2 is installed
Add a switch that allows toggling between inline and two-column format
when Wikidiff2 is installed.

Inline toggle should also support no-JS toggling

The legend should toggle when switching from table to inline after the
first load.

Introduced a temporary feature flag to show this inline toggle so that
we can merge and easily continue the improvements. It will be removed
when ready for production.

Bug: T336712
Bug: T330229
Change-Id: Ie6a48e495f2bb299d8b984e7c40363d534c7915b
2023-06-26 10:31:54 +10:00
Tim Starling
512806564d Do not generate diffs for slots with identical content
When the old and new revisions have identical content in a given slot,
do not include the relevant SlotDiffRenderer in the return value of
DifferenceEngine::getSlotDiffRenderers(), so that the relevant table
prefixes and modules will be omitted.

Also:

* In TextSlotDiffRenderer::getTextDiff(), return an empty string when
  the inputs are equal, instead of invoking the diff engine. Previously,
  the inline format would produce a <tr> element in this case, causing
  the diff-empty message to be omitted.
* Clarify the doc comment up the stack, indicating SlotDiffRenderer may
  return zero <tr> tags, it doesn't have to be one or more.
* Fix the documented return type of getSlotContents() so that @var is
  not needed in the caller.
* Don't convert the return value of SlotDiffRenderer::getDiff() to
  boolean. It's always a string so compare it to the empty string.

Note that the cache key for inline empty diffs will change, because
TextSlotDiffRenderer::getExtraCacheKeys() is not called anymore for
empty diffs.

Bug: T338670
Change-Id: I0774ba0b159ac43ec214403cf2d06740f6d067cd
2023-06-21 14:46:19 +10:00
Tim Starling
359ce91a12 Move DiffEngine and helpers to includes/libs/Diff and put them in a namespace
I will address the dependencies on MW core in a followup.

Bug: T339184
Change-Id: I892364b0c9f15c9de4cfc29c683670c172d71764
2023-06-20 15:15:13 +10:00
Sam Wilson
1eb586013c diff: Add legend and tooltips to inline diff display
Add a legend at the top of the inline diff display, showing the
meanings of the colours of the inserted and deleted highlighting.
Also add the same text as tooltips on the highlighted elements.

The legend is added as part of a new area above the diff table
that can be modified via a new TextSlotDiffRendererTablePrefix
hook, so that extensions can add other buttons etc. there as
required.

This is a follow-up to the previous attempt, which added the
legend in DifferenceEngine::showDiff() and was called from
too many places. This patch moves it to be called in
DifferenceEngine::showDiffPage().

Bug: T324759
Change-Id: I2a3c67bcfa47313dee597e602a62073e4e298cd2
Follow-up: I6de30bf79eb5ac262285951792782b870d075e00
2023-05-31 15:43:28 +10:00
Matěj Suchánek
e47c441078 Fix many typos in comments
Found using IntelliJ's "Typo" code inspection.

Change-Id: I746220ebe6e1e39f6cb503390ec9053e6518cf16
2022-05-10 12:46:11 +00:00
Umherirrender
b126dbe3f2 Fix various documentation related to null types
The functions returning null or the class property is set explict null

Found by phan strict checks

Change-Id: I4a271093fb6526564d8083a08249c64cb21f2453
2022-02-26 10:31:24 +01:00
Umherirrender
44fd53fee3 Using @return never documentation on always-throw-function
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before

Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
2021-09-07 17:29:03 +02:00
TChin
077b9af960 Replace Assert::parameterType with typehints
Bug: T287530
Change-Id: I6060e194339614b53e3a9c036ff3a3ac2e68f8df
2021-08-03 10:03:48 -04:00
jenkins-bot
6ca4b5320c Merge "Fix error handling in TextSlotDiffRenderer::getTextDiffInternal()" 2021-03-04 10:24:03 +00:00
Umherirrender
8de3b7d324 Use static closures where safe to use
This is micro-optimization of closure code to avoid binding the closure
to $this where it is not needed.

Created by I25a17fb22b6b669e817317a0f45051ae9c608208

Change-Id: I0ffc6200f6c6693d78a3151cb8cea7dce7c21653
2021-02-11 00:13:52 +00:00
Umherirrender
692148fd62 Improve and add documentation to diff related classes
Change-Id: Ic10c700e970018fa0b9a219d96d56908b0298d71
2020-11-22 19:09:02 +00:00
DannyS712
94169ee873 Whitespace cleanup: Use tabs for indentation, avoid double spaces
Change-Id: I346073b59d283029bd6666356c62c81e687ea5e6
2020-06-27 07:53:07 +00:00
Gergő Tisza
4612a694d8
Fix error handling in TextSlotDiffRenderer::getTextDiffInternal()
A long time ago, returning false from the predecessor of this
method (done on filesystem error when trying to generate a diff
via an external executable) meant the diff result was treated as
an internal error and not cached, but I2f8a9db broke that.

There is not much point in trying to fail softly on tempfile
creation errors anyway, so just replace it with exception throwing.

Change-Id: Idb15a0e1f67cb1ccbf7c47788f4eecbaa8176665
2020-06-24 11:18:59 +02:00
Reedy
988174411c Fix even more PSR12.Properties.ConstantVisibility.NotFound
Change-Id: If1335359b545c36fc979676c3e88d87628f9389e
2020-05-16 00:51:31 +01:00
ArtBaltai
30e54b3962 Introduce ContentHandlerFactory
Added:
- ContentHandlerFactory
Tests:
- PHPUnit
Changed
- Calls of changed and deprecated
- DI for some service/api
Deprecated:
- ContentHandler::* then similar to ContentHandlerFactory
- ContentHandler::getForTitle
- ContentHandler::$handlers

Bug: T235165
Change-Id: I59246938c7ad7b3e70e46c9e698708ef9bc672c6
2020-02-07 00:53:51 +03:00
James D. Forrester
0958a0bce4 Coding style: Auto-fix MediaWiki.Usage.IsNull.IsNull
Change-Id: I90cfe8366c0245c9c67e598d17800684897a4e27
2020-01-10 14:17:13 -08:00
jdlrobson
1c2de3d397 Inline diffs upstreamed from MobileFrontend to core.
A new inline mode is provided for diffs.
It is only available when wikidiff2 is installed.
There is no PHP implementation (one can be added later if
necessary)

For now, it is accessed by passing diff-type as a query string parameter
e.g ?diff-type=inline
A control for switching between the two is added as a follow up.
see Ie9bb17789d90b7492559782021937f3f3e4356f8

* The final method getSlotDiffRenderer now takes a second parameter
options
* The method `getSlotDiffRendererInternal` is deprecated and
replaced with the more flexible `getSlotDiffRendererWithOptions`
This has potential to be a breaking change but is unlikely to impact
any existing clients.

Note: PHP implementation can be added later if necessary

Bug: T117279
Change-Id: I4f81c8ccf253dd4aa6cf43c3fad257b4b0dd1ebd
2019-12-06 01:08:10 +00:00
Daimona Eaytoy
c659bc6308 Unsuppress another phan issue (part 7)
Bug: T231636
Depends-On: I2cd24e73726394e3200a570c45d5e86b6849bfa9
Depends-On: I4fa3e6aad872434ca397325ed7a83f94973661d0
Change-Id: Ie6233561de78457cae5e4e44e220feec2d1272d8
2019-09-03 17:19:21 +00:00
daniel
6906a7728c Add UnknownContentHandler.
UnknownContentHandler can be configued to handle models that
belong to extensions that have been undeployed:

  $wgContentHandlers['xyzzy'] = 'UnknownContentHandler';

This way, no errors will be thrown when trying to access
pages with the unsupported model. Instead, an error message is
shown, and editing is prevented.

This patch also improves handling of non-editable content in
EditPage and in DifferenceEngine.

Bug: T220608
Change-Id: Ia94521b786c0a5225a674e4dc3cb6761a723d75b
2019-08-29 10:43:11 +00:00
Adam Wight
2eec94991b Deprecate $wgWikiDiff2MovedParagraphDetectionCutoff
Bug: T194272
Change-Id: I174192dc2b91409e023c06b054bc1bba8bfc571f
2019-05-22 15:42:20 +02:00
Fomafix
14d9e80ba4 Fix order of @var parameter in PHP
Replace

 @var $foo type

by

 @var type $foo

Change-Id: Iec8b90ffea4028a1005daef422b7c4ec38c2715f
2019-04-08 18:37:56 +02:00
WMDE-Fisch
cbe4ffe26d Remove warning for unnused 4th argument on wikidiff2
Since we changed the signature back to 3 arguments in wikidiff2 verion
1.8.0, the warning is invalid.

Bug: T220217
Bug: T203069
Bug: T194272
Change-Id: Ia326c67de28a4e9b024466c62097b4e1e1096007
2019-04-05 18:27:15 +02:00
Thiemo Kreuz
c3dfa88966 Add missing empty lines between methods
This might hint at an edge-case in the PHP CodeSniffer sniff that should
detect if methods are separated by a single empty line. Feel free to
investigate. I, personally, can't invest more time in this than
suggesting this quick fix.

Change-Id: Ib3c60eac76f255b4fe929f7933de256222716576
2019-01-15 19:14:35 +00:00
Gergő Tisza
ec72a9f495
SlotDiffRenderer: add utility method for parameter type checks
Change-Id: I0161070fd0330d4945cec2f76f4fd8128a9793b9
2018-09-24 23:09:18 -07:00
WMDE-Fisch
763129619c Prepare DiffRenderer to changed parameters for wikidiff2 1.8.0
The config parameter handling in wikidiff2 will migrate completely to
phpini values. Due to that we will remove the php config variable
wgWikiDiff2MovedParagraphDetectionCutoff and its usage.

In the process of deleting the 4th parameter in the wikidiff2_do_diff()
method, this exception is added so deployment can be handled separately.

On the long run these exceptions will be removed completely and the
(then) current mediawiki will depend on wikdiff2 > 1.8.0

Bug: T194272
Depends-On: I673d2489d5bce1a24a6ea83e168704948564661b
Change-Id: I30262412b0784b84af88ffab6ed0694a08b671e9
2018-08-23 15:37:29 +00:00
Gergő Tisza
d31580eeb0
[MCR] Render multi-slot diffs
Move logic for rendering a diff between two content objects out of
DifferenceEngine, into a new SlotDiffRenderer class. Make
DifferenceEngine use multiple SlotDiffRenderers, one per slot.

This separates the class tree for changing high-level diff properties
such as the header or the revision selection method (same as before:
subclass DifferenceEngine and override ContentHandler::getDiffEngineClass
or implement GetDifferenceEngine) and the one for changing the actual
diff rendering for a given content type (subclass SlotDiffRenderer and
override ContentHandler::getSlotDiffRenderer or implement
GetSlotDiffRenderer). To keep B/C, when SlotDiffRenderer is not overridden
for a given content type but DifferenceEngine is, that DifferenceEngine
will be used instead.
The weak point of the scheme is overriding the DifferenceEngine methods
passing control to the SlotDiffRenderers (the ones calling
getDifferenceEngines), without calling the parent. These are:
showDiffStyle, getDiffBody, getDiffBodyCacheKeyParams. Extensions doing
that will probably break in unpredictable ways (most likely, only
showing the main slot diff). Nothing in gerrit does it, at least.

A new GetSlotDiffRenderer hook is added to modify rendering for content
models not owned by the extension, much like how GetDifferenceEngine
works.

Also deprecates public access to mNewRev/mOldRev and creates public
getters instead. DifferenceEngine never supported external changes to
those properties, this just acknowledges it.

Bug: T194731
Change-Id: I2f8a9dbebd2290b7feafb20e2bb2a2693e18ba11
Depends-On: I04e885a33bfce5bccc807b9bcfe1eaa577a9fd47
Depends-On: I203d8895bf436b7fee53fe4718dede8a3b1768bc
2018-08-20 15:39:12 +02:00