Commit graph

126 commits

Author SHA1 Message Date
sbailey 511543e3f1 Add refname parameter to cite_error_empty_references_define error
Bug: T51538
Change-Id: I2850b7f181f44465437bc486bc544c5cd58aa5e3
2020-12-21 13:31:37 -08:00
sbailey b9b10a3fe0 Add group name to Cite error cite_error_references_group_mismatch
Bug: T51538
Change-Id: Ie6e04edcdf4b9760711ec53021d65970691a3813
2020-12-18 22:16:28 +00:00
sbailey 7bd9f87157 Add parameter $refName to Cite error cite_error_ref_duplicate_key
Bug T51538
Change-Id: If8399be12a5cad025b3a4db8e970c8de96c75ad6
2020-12-16 13:58:45 -08:00
sbailey 5fbf890f12 Add direction parameter to cite_error_ref_invalid_dir message
Bug: T51538
Change-Id: I5e964ad7341a46552d7b8eded0d844c0132816b1
2020-12-16 20:24:57 +00:00
sbailey cf4a49ba6e Add group name parameter to cite_error_group_refs_without_references
Bug: T51538
Change-Id: I8708ffa21c2ef68c124a5b055a6860cfb4ec12e1
2020-12-16 20:19:33 +00:00
Arlo Breault d95a783cc8 Stop referring to spec version numbers where unnecessary
Presumably the source should be up-to-date with the latest spec.

Change-Id: Iaea7f80e9d3bbd3520a7b499252162240deeba62
2020-12-16 13:55:27 -05:00
Subramanya Sastry 07bcfd9add Purge Sanitizer proxying from ParsoidExtensionAPI
Sanitizer is heavily used by extensions and we decided to let
extensions directly access it.

So, stop proxying those methods from ParsoidExtensionAPI.

Change-Id: I5ff285bf33733878135e2091d53ae12f7340c8fc
2020-12-10 16:54:30 +00:00
sbailey e0322afd84 Parsoid Cite add class mw-ref-follow for refs with follow
* Addresses a FIXME (T263052) where Parsoid Cite injects
   style = "display: none;" in refs with follow instead of
   having css do that triggered by having a class "mw-ref-follow"
   as part of the refs html.

Bug: T263052
Depends-On: I351516b81566aba0adb4d298e39806dfb4fc7b03
Change-Id: I8bfc4ee3df162e2040e3c6f0c37fbf2a7c30d7f6
2020-12-10 16:54:25 +00:00
Arlo Breault 8d4543954f Cast references attributes to strings
Follow up to 01cf61a

Numeric array keys are returned as integers.

echo "<references 2/>" | php bin/parse.php

Bug: T269748
Change-Id: I892753c330f95d258e0310626f109386fd020177
2020-12-09 16:05:12 +00:00
Arlo Breault 3c15454851 Refine adding module(style)?s in extapi
Bug: T269022
Change-Id: Ic2c56c554934ced2aea04317d988098ca840076f
2020-11-30 17:15:27 -05:00
Arlo Breault 6525d69200 Reconcile some ref errors cases with $hasFollow
Change-Id: I5e3a27366f177af6c221d57da6e31f28cc91bb0c
2020-11-25 13:51:37 -05:00
sbailey de5d806335 Cite error tag name defined in references not used before
Bug: T51538
Change-Id: Id89b3cc186de42e5e5c05f15d7546db9d64ec864
2020-11-25 13:50:25 -05:00
sbailey 703dc8dc05 Adding cite error ref in reference with mismatched group
Bug: T51538
Change-Id: I5492dbaebb7bca79e83be09fdcfe810eaef8c053
2020-11-24 17:22:56 +00:00
Arlo Breault 680df4379c Use inReferencesContent flag to get rid of processRefsInReferences
It's sufficient to handle this case in processRefs.

Also moves $referencesGroup to the ReferencesData instance, rather than
passing it around as a variable (inconsistently).

Change-Id: I8637e3ce644642259e353d0df3d9c0dbc3102c7b
2020-11-24 17:22:01 +00:00
Arlo Breault b88f9ca881 Fix porting bug from 005176a
Bug: T249742
Change-Id: Iabe86266c06b2cbc3c51b16b73d360a7182878f1
2020-11-24 10:54:23 -05:00
sbailey 4c7108f553 Adding cite error ref in reference no content defined
Bug: T51538
Change-Id: I4cdcf1a36f472f582812dbb5e7050c0ead614639
2020-11-23 18:32:26 -05:00
sbailey 1f0221e327 Add reporting of cite error of a ref in reference without name specified
Bug: 51538
Change-Id: I193d5583b31be32741088fb25c348878f34b5016
2020-11-23 23:30:14 +00:00
Arlo Breault e3ca32c9ff Add method to check if in references content
More specific than just embedded content, needed for adding errors in
follow up patches.

Change-Id: I4bf659cd208c3322870e3ea0126bda4a2a7037d8
2020-11-23 18:53:03 +00:00
Arlo Breault b664b64fcb Use $extApi->pushError for invalid references parameters
Follow up to 01cf61a

Change-Id: Ic4483f151d12352cc9e6f6094e4df442eabca376
2020-11-16 22:19:54 +00:00
Arlo Breault 2f1bbc1804 Only look for data-mw.body.id in the top level dom
Follow up to 6c15f6e where the same approach was taken in dom diff'ing.

Clarifies where the "id" is expected to point and the limitations of the
approach vis-a-vis embedded content.

For example,

<ref>hi ho</ref>
[[File:Test.png|<references />]]

won't roundtrip, and never did, because the references section the "id"
would point to is in embedded content.

This was really only ever about the case where the <ref> itself was
found in embedded content, like an image caption, and we wanted to find
a top level references section, like,

[[File:Test.png|<ref>hi ho</ref>]]
<references />

The one case old approach was ostensibly doing something smarter was if
both the references section and the ref were in the same embedded
content, as in,

[[File:Test.png|<ref>hi ho</ref><references />]]

However, at least for file captions, those were always serialized in a
fragment of the top level doc and suffer from same dropping as the first
example here.  Maybe some other embedded content is handled differently,
in which case this is probably an acceptable regression.

Change-Id: Ia90eadcc5099a8c27f0bf3fda0ce2f0effca7bcc
2020-11-10 21:56:16 +00:00
sbailey 01cf61ad67 Adding check for illegal attributes in references tag
Bug: T51538
Change-Id: I7dbc577a61abb660d2bdb66ead0d7b71fd66cf47
2020-11-10 19:47:04 +00:00
Arlo Breault 4310b6a243 Mark up cite errors in embedded content
It's a feature of named refs that we only know at the time of inserting
the references list whether they have content or not, and are therefore
in err.  The strategy of 4438a72 was to keep pointers to all named ref
nodes so that if an error does occur, we can mark them up.

The problem with embedded content is that, at the time when we find out
about the errors, it's been serialized and stored, and so any pointers
we might have kept around are no longer live or relevant.  We need to go
back and process all that embedded content again to find where the refs
with errors are hiding.

This patch slightly optimizes that by keeping a map of all the errors
for refs in embedded content so that only one pass is necessary, rather
than for each references list.  Also note that, in the common case, this
pass won't run since we won't have any errors in embedded content.

Bug: T266356
Change-Id: I32e7bfa796cd4382c43b3b1d17b925dc97ce9f7f
2020-11-06 18:31:26 -05:00
Arlo Breault b2e2732674 Switch some uses of matchTypeOf to hasTypeOf in Cite
Change-Id: I99986c337944547ae398851676de13377f4114b1
2020-11-06 13:14:04 -05:00
Arlo Breault c675396445 Fix adding 'cite_error_group_refs_without_references' to unnamed refs
Follow up to 02fb17d, which was only iterating over named refs.

Bug: T51538
Change-Id: I1a1ce39029c2e9e6e29e768675bcde266ccf3247
2020-11-06 13:14:03 -05:00
Arlo Breault 049735ba0e Clean up signatures of ref group accessors
No need to hedge on null.

Change-Id: I2afb7619a113d784741bd7d29eccf4d8368fe56f
2020-11-06 17:45:18 +00:00
Arlo Breault 0254f138ab Suppress linkbacks for all refs in embedded content
Not just for refs in references content, since they'll be equally
inaccessible everywhere.

Change-Id: Id0a2361b41d9b8103e011ff4f809fa0809169bb3
2020-11-06 17:45:16 +00:00
sbailey 095b2c2388 Add "reference" class to <sup> in addition to the "mw-ref" class
* Added, required adjusting many tests and knownFailures info.

 * Change impacted parserTests.txt as well, updated.

Bug: T265930
Change-Id: I79d915ebd811bdb83344bf7295862588fe7d46ea
2020-11-05 00:17:54 +00:00
Arlo Breault 1dda4cdc8a Consolidate adding ref errors at references insertion
Change-Id: I01ce55989fb7b822320c63ddad19c2edf7e03bf9
2020-10-29 15:54:30 -04:00
sbailey 02fb17d102 Refs in group with no <references group="xxx" />
* Generating error message in data-mw in affected refs

 * Cite test validates correctness of adding error to afflicted
   refs

 * Autogenerated references aren't considered erroneous
   (the extension to the legacy parser also generates them)
   and are not suppressed when serializing because apparently
   that's the behaviour Parsoid clients want.  However, in
   this patch we're marking up autogenerated references
   *with group attributes* as errors (the legacy extension
   doesn't generate them at all) and are choosing to suppress
   them when serializing since we considered them an error
   while parsing and don't want them to persist in the content.

Bug: T51538
Change-Id: Ia651b10449dc41c2cb439b33a361e8c8e482f502
2020-10-28 22:28:13 +00:00
Arlo Breault b5b3475d28 Set a top level doc when serializing
First step in using a single document for the serializing direction as
well.

Bug: T265061
Change-Id: Ia845faed693bf4c6f74facc7576d01a9f5a875be
2020-10-28 18:48:17 +00:00
Arlo Breault 568034a00c Traverse with inEmbeddedContent for ref in ref
While, for the most part, content nested in refs will end up in the
references section and should be fine to maintain a linkback.  However,
if content differs from a previously named ref (ie. !== cachedHtml), it
ends up being serialized and nested in data-mw, so is also unsafe.

Bug: T266294
Bug: T266356
Change-Id: Ia92f42e06353c411b986d0665cbe6338052555fa
2020-10-23 17:52:51 -04:00
Arlo Breault 8e237b4e34 Make $inEmbeddedContent an explicit stack
Change-Id: I48ff2f7be352fdec72b2c5e0eeee843330ec3872
2020-10-23 11:42:45 -04:00
Arlo Breault b8d4bb1f37 Set inEmbeddedContent when running processRefsInReferences
In 47506af, serializing references content to be added to data-mw was
delayed until we were inserting the reference into the dom, to give us a
chance to mark up errors about not finding named ref content.  After
which, the content is cleaned out of references to make room for list
items that belong there.

In 6bd0594, we noted that we can't store linkback from embeded content
since the content is released after serializing and won't be available
when it comes time to mark it up with errors.

Similarly, the linkbacks added to other groups from the references
content won't be available after inserting that reference and so we
shouldn't be holding on to them either.  This means that we won't be
marking them up for the named error above but with the benefit that we
won't crash when trying to access these linkbacks from another group.

A test case is added which crashes trying to access a linkback from a
group that has already been inserted.

Note that the extension to the legacy parser considers referencing
another group from group content to be
"cite_error_references_group_mismatch".  If we choose to also do that,
we may be able to reverse this change.

Change-Id: Idf0e49fa07dc3614068793c72a30ce3de1e2392c
2020-10-23 11:42:25 -04:00
sbailey a8e299b21e Added cite error checks for numeric digit in name
* Note: the <ref name="1"> numeric is flagged as an error, but
   <ref follow="1"> is not flagged as well as it would be duplicitous.
   We are proposing that Numeric digits as a name be changed to
   a warning as Parsoid Cite handles them properly without error.

   Background: The core Cite extension started off not allowing numeric
   names because the data structure they used made it inconvenient,
   but Parsoid Cite has a separate index for named refs.
   The core Cite devs thought there was potential for a conflict in
   the ids if numeric names were allowed. The ids that core uses
   follow the pattern:
     for no name defined:  cite_ref-1             cite_ref-2
     for name="refname":   cite_ref-refname_3-0   cite_ref-refname_3-1
   so for name="1" at worst you might see ids like:
     cite_ref-1  or  cite_ref-1_2-0
   so that does not produce conflicting IDs and isn't a concern,
   and that is the pattern that Parsoid Cite uses.

 * Error case of numeric in name and follow and two tests that
   validate errors.

Bug: T51538
Change-Id: I95d725d0f77abadc1ddb2dd6939762b7d322e4f2
2020-10-15 23:57:16 +00:00
sbailey aadef667d4 Added cite error checks for invalid text direction
* Error case of dir= not ltr or rtl is caught and a cite tests
   validates the new check.

Bug: T51538
Change-Id: I8c7e088416f0a000e638771a3fe5e8e0c58bcc23
2020-10-07 13:22:46 -07:00
Arlo Breault 1ac603ff1c Always wrap follows content
Avoids crashers from trying to serialize the named content twice if
there's a valid follow with some other error, as expected in the FIXME.

This introduces a backwards incompatibility for invalid follows, which
will result in the contents of the ref being dropped.

A test is added to assert that selser will save us for the most part.

Change-Id: I1f572f996a7c2b3b852752f5348ebb60d8e21c47
2020-10-07 11:42:23 -04:00
Arlo Breault 6bd0594f28 Don't keep pointers to nodes from embedded content
Since the fragment they're subtrees of goes out of scope.

Follow up to 2f09cdb

Previous to that patch this wasn't an issue because we were creating a
whole document which is retained by the environment.

Fixes the warnings from,
"PHP Warning: DOMElement::getAttribute(): Couldn't fetch DOMElement"
https://logstash.wikimedia.org/app/kibana#/doc/logstash-*/logstash-2020.10.02/parsoid-tests?id=AXTqaLL12lgCwKx7fVYz&_g=h@a06543d

Tested on scandium with,
node bin/roundtrip-test.js --proxyURL http://scandium.eqiad.wmnet:80 --parsoidURL http://DOMAIN/w/rest.php --domain vi.wikipedia.org "Vua_Việt_Nam"

Change-Id: I74bc7de79b18054e19b77af25e978d3ab3a505e4
2020-10-02 15:57:33 -04:00
Subramanya Sastry f00325d6cc Introduce preprocessing in the HTML -> WT direction
* This patch introduces a preprocessing step on the edited DOM.

* Existing preprocessing code has been extracted into the
  preprocessDOM method.

  Any registered extensions preprocessors are invoked on the DOM.
  So, this assumes that the htmlPreprocess extension listener is only
  applicable to the edited DOM. If we want to expose the concept of
  selective serialization through the API, we may want to add an
  additional interface method / listener to the DOMProcessor class.

  As of this patch, this is somewhat theoretical since there are no
  such extension handlers registered on either DOM. Future patches
  can clarify this better as specific needs arise.

* The handler also calls the serializer's custom preprocessing steps.
  This step is applicable to both the original as well as edited DOM
  (since DOM Diff is impacted by the results). If a need arises,
  in the future, we may introduce a new extension DOM processor method
  that applies to both original and edited DOMs.

* Right now, only selser strips section tags and non-selser wts
  doesn't need to. So, preprocessDOM there is empty. Additional
  selser-only DOM preprocessing will show up in later patches.

* Moved a stub HTML->WT preprocessor in Cite extension to RefProcessor.

Bug: T254501
Change-Id: I0c12afb2ea82617406d72ad872ac4f33678fa5f2
2020-09-30 17:12:45 -05:00
Arlo Breault 0667a01637 Release fragments once they're no longer needed
These should all be empty at this point anyways.

Bug: T230861
Change-Id: If53d12c8b8e83cea2e86cb8d6107c22aa5724dd6
2020-09-29 22:37:21 +00:00
Arlo Breault 2f09cdb732 One document to rule them all
The description in T179082 suggests that by using one document for the
entire parse, we'd probably see some performance gains from not having
to import nodes when we get to the top level pipeline and we'd avoid the
validation errors from 19a9c3c.

However, the spec seems to suggest creating a new document when parsing
an HTML fragment,
https://html.spec.whatwg.org/#html-fragment-parsing-algorithm

And, indeed, domino implements it that way,
12a5f67136/lib/htmlelts.js (L84-L96)

So, the request in T217705 may be a little misguided.

What then is this patch good for?  In T221790 the ask is that
sub-pipelines produce DocumentFragment which make for cleaner interfaces
and less confusion when migrating children.

The general outline here is that a document is created when the
environment is constructed that gives us the 1-1 correspondence.
Sub-pipelines do create their own documents for the purpose of tree
building, as in the fragment parsing algorithm, but are then immediately
imported to DocumentFragments to be used for the rest of the
post-processing passes.

Bug: T221790
Bug: T179082
Bug: T217705
Change-Id: Idf856d4e071d742ca38486c8ab402e39b3c8949f
2020-09-29 22:36:33 +00:00
Arlo Breault e3484acfc6 Highlight when we have a valid follow
Rather than using no errors as proxy.

Change-Id: I78c445838de2d4f5f6a0f17e5bb38996674ca999
2020-09-17 18:13:08 +00:00
sbailey deb9451c15 Removed parsoid cite extension follow ref id's
* Remove the id's from follow refs because they were
   duplicating the same key value erroneously and also
   did not provide useful info. Fixed all tests accordingly.

 * Added FIXME which refers to a new Phab ticket about
   removing the code which adds style = display-none
   that will be moved to CSS at some point.

Bug: T262986
Change-Id: Ib59f5eec951aa83a02357de865df8ab3dd8d2f67
2020-09-17 16:50:29 +00:00
Arlo Breault bb34d30839 Follow up to "follow" functionality for Cite
These refs get a `style="display: none;"` since they're
not intended to be user visible.

Follow refs with errors conform to the proposed spec in T251842

Bug: T51538
Change-Id: Ie4ea28e7f9afde24614874bb4b8e07c5cabafa12
2020-09-10 12:41:06 -04:00
sbailey 467b82701b Adding "follow" functionality to the Cite extension
* Interim state commit with experimental code.

 * Updates to citeParserTests.txt to check now valid follow
   functionality and newly passing tests.

 * Added to follow refs, <sup style="display: none;" about=...
   to suppress display of hidden sups needed for VE to use
   in editing follow refs.

 * Added code to implemented follow functionality and catch
   invalid usage.

Bug: T51538
Change-Id: Ic3ac8237fd2c490cfaf2fe799759742f72f10686
2020-09-09 19:25:14 -04:00
Arlo Breault 46a9900f69 Implement DOMCompat::replaceChildren()
Change-Id: Id2597e403dc2cda0804005d5e615f94c965a6196
2020-09-03 12:01:02 -04:00
sbailey 5e5e360ffd Fix for missing content check where ..body->extsrc is undefined
* Bug fix for accessing undefined extsrc member variable in edge
  case. See T260082 for deeper explanation of the WT that caused a
  case where empty flag is not set and extsrc is also missing, but
  since either case including extsrc being unset indicates no
  content, this additional check is safe for now.

Bug: T259676
Change-Id: I20750c6977883668c83bdae78fbeb171f899e1ab
2020-08-10 22:02:24 +00:00
Subramanya Sastry 542bd7fb99 Extension config option: Rename sealFragment to unpackOutput
* The latter feels more readable and intuitive.
* The option defaults to true.
* To make for simpler code, I ensured that the option value is always
  set before it is accessed.
* But, the typeof value still uses the "/sealed/" qualifier.
  Alternatively, I could use "/packed/" if we want to adhere to the
  config value name more closely.
* Tangentially related changes:
  - made getWrapperTokens a private method since it is only used
    internal to PipelineUtils.
  - remove default value for $opts in encapsulateExpansionHTML since
    the value is always passed in everywhere.

Change-Id: I86c4e5adf11e3151f51f2623e5ed85282a2e1298
2020-08-06 18:33:06 +00:00
Arlo Breault d6bcc0ef14 Prefer nullable types in comments
This was done with a custom sniff in,
MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php

`$singleType === 'null' && count( $explodedType ) === 2`

since there's some ambiguity with,

`what|type|null`

but also a case like the following is left out,

`string[]|null`

Change-Id: I1bd50a4486d7ef4974280b476fd03d3ee53232b3
2020-07-29 14:24:32 -04:00
Arlo Breault cdf8ac149b Wrap extension token so that it won't be exposed
Alternative to I6ea271a5d5c7b12a13bb12a682c39bcfd7b1f116

We can follow this up by passing the ExtensionTag to the
ExtensionTagHandler constructor.

Change-Id: I5b1b191bc85968ad617eb3ebcdd7721c55006af2
2020-07-23 14:51:24 +00:00
sbailey e29b51ebcc Match core error key for self-closed ref without name
* Bug fixes to accurately match core cite use of
   cite_error_ref_no_key error and adjusted citeTests to match.

Bug: T51538
Change-Id: I3ae5300a5f86decebb7e67c5ea57c0c15677fbcc
2020-07-02 19:15:18 -04:00