* 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
* Error case of dir= not ltr or rtl is caught and a cite tests
validates the new check.
Bug: T51538
Change-Id: I8c7e088416f0a000e638771a3fe5e8e0c58bcc23
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
* 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
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
* 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
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
* 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
* eslint-config-wikimedia: 0.16.2 → 0.17.0
The following rules are failing and were disabled:
* no-shadow
* grunt: 1.2.1 → 1.3.0
Change-Id: I2ee910e2940c3db520531b91329955d01f2a9076
This is done to make the discussion in If3dcfd7 easier.
When we introduced this code we actually used it to format
entire numbers. We had to change this later to *not* localize
digits, but only separators. Language::formatNum is and always
was able to do this, so we just continued to use it.
This is discussed now.
It turns out there is only a single place left where we use
formatNum, and it does nothing but localizing the decimal
point. There is another way to do the same.
Bug: T237467
Change-Id: I89b17a9e11b3afc6c653ba7ccc6ff84c37863b66
* 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
* 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