It is about time to implement a generic fix for T214994 which will
capture the Cite-specific scenario we are tackling here in bits
and pieces. We have fragments of a T214994 solution spread in
Utils/ and here while tackling shiftDSR and convertOffsets
for Parsoid/PHP. So, someone needs to take a look at all these
pieces and implement the necessary abstraction.
As part of that, maybe we also need to revisit our embedded HTML
data-* representation so there is a uniform way to inspect these
attributes rather than every semantic element (template, extensions,
lang variants, media) choosing their own custom embedding scheme
which makes discoverability harder and error-prone.
Change-Id: Ifd19d2d8d20dbd0dda0fa1cc338a07afb37c4213
This managed to catch a few bugs where there is likely coverage gaps.
The regenerated plugin list again comments out a few that generated too
much noise but may be worthwhile follow ups.
Change-Id: I746abd0f1406b4b7ffa497afdad3939fe437c25d
* This is an instance of a bigger issue that we need to look closely
when we are integrating Parsoid with core parser.
Bug: T235656
Change-Id: I3d652727293461c7968e83be8994ba0572bae8e4
* Though this doesn't immediately affect anything, it just makes
usage a bit more consistent.
* A followup patch that fixes gaps in shiftDSR code will now be
able to reference the html property as dmw->body->html to match
html2wt usage.
Change-Id: I9dfcd9d40205f6e64e139bf3f75a322915af3232
* When a page is missing an explicit <references /> tag, we insert
an implicit <references /> tag and assign it a zero-width DSR with
a starting offset equal to the length of the page. However, now that
we have byte offsets, that should have been strlen, not mb_strlen.
This was causing incorrect DSR assignment on this implicit tag
and causing trailing newline selser diffs on these pages.
* Debugged on this reduced test case: "* a – b <ref>x</ref>\n\nc\n\n"
and comparing selser trace and then DSR offsets on the DOMs.
Change-Id: I8aebf307197935259df78251fb4a26c593f29603
Tim Starling has indicated in couple different places that end(..)
is not preferred and he had implemented a private version of lastItem
in the PEG grammar code whereas PHPUtils::lastItem was recommending
use of end(..).
In this patch, I moved the implementation from the grammar to PHPUtils
and replaced end(..) with PHPUtils::lastItem in a number of places in
the codebase. We should discuss whether we want to use this helper
everywhere.
Resolved a couple of PORT-FIXMEs in the bargain.
Change-Id: I837f2a98003df8ab7dbdf9af045e17bdd6e27799
* We could potentially also exclude regexps for node name checks
* A few additions looks like could potentially have caused subtle
failures in edge cases.
* Unrelated changes: Used # regexp terminator in a number of regexps
to eliminate escaping of / character.
Bug: T231980
Change-Id: Ie2451349684c248d93e064e3e7009d0d2d60acf3
The extension handler expects a `null` for this, `false` is an
indication not to use the toDOM handler.
php bin/parserTests.php tests/citeParserTests.txt --filter "CircularRef" --wt2html
Change-Id: I849a9aca1133f8a793c9d77e05f192a6af5d78f9
Introduce PHPUtils::arrayToObject() which duplicates the array before
converting it to an object. Workaround for
https://bugs.php.net/bug.php?id=78379
Bug: T228346
Change-Id: If9ef35e9e5183117025bc9cd705b695f270aa244
Two classic PHP errors
- string '0' is a falsy value in if conditions and so we need an
explicit === '' check for empty strings
- arrays need to be passed by reference to capture modifications
in callers.
Change-Id: I07d0e39c44a923ac1faeb2de01433e951c3de914
* Don't unconditionally run fromHTML and before handlers without
checking if we have a native extension handler.
* Remove unintential implementation of `before` in Ref.php
* Hybrid tests for Cite now passes again.
* Discovered while running native parser tests and isolated those
crashers to this.
Change-Id: I45b48b595a5aee2b8b8d00b4ebcf73a5ea7bc8a3
Restructure ExtensionTag as an abstract class with default do-nothing
implementations of all methods. So instead of Translate and LST
not implementing ExtensionTag::toDOM, they inherit the default
implementation which returns false, and that has the same effect.
The intention is to move SerialHandler::fromHTML and SerialHandler::before
into this framework as well. Every "optional" method should have a default
implementation in the base class which returns false.
Change-Id: I0ad5c714601c0cf0b5189d4d282c67c6b53fc760
Passing srcOffsets which don't actually correspond to actual regions of
the source wikitext cause problems in the token offset conversion code.
Instead, parse the wikitext as itself, then adjust the TSRs in the DOM
tree.
Since Gallery isn't ported to PHP (yet), update the
automatically-generated Gallery/index.php. The newly-added
ContentUtils::shiftDSR() was ported, however.
Change-Id: I28f3d3398930733ae2bcf9759e49c45f93bc7190
* Now that we are in sync land, we don't need to catch exceptions
and log error messages at multiple places. Let them bubble up
to the top.
* I noticed this was actually getting in the way of debugging because
with $env->log unimplemented, I was only getting very generic
failures instead of the root cause that was being suppressed
and unlogged.
* There are still a couple of places where we have generic Exception
catching in place where it does make sense currently. For example,
we aren't interested in what caused a templatedata fetch to fail.
We simply fall back to regular serialization - the rationale here is
that it is better to emit a transclusion without the preferred formatting
(but syntactically correct) instead of losing the edit altogether.
* Minor unrelated fix in Cite/Ref.php: Use !isset() instead of empty()
Change-Id: Iebff6f37dcd8278185c4a74b72a99b528efa20ff
* Source offset fixes: followup to 31d356a5
- there were instances of $tsr[0] and $tsr[1] that hadn't
been converted over to $tsr->start, $tsr->end
- removed dead code
* Cite fixes: followup ot 005176a3
- Fixes array / object mixups
- Bug fix
* html2wt/WikiLinkHandler fixes
- Protect access to missing properties in data-mw opt list
* Other assorted fixes
- Added missing typehints and improved doc types
- Simplified some code patterns
- Cast extension attributes to object since that ends up
in data-mw which is a stdclass object.
Change-Id: Idd04b0d3819be3660823047a90330fd1213388cf
* All wt2wt, html2wt, and all but one html2html tests pass in
hybrid mode when entire html2wt code is run in PHP
Set "Serializer: true" in the html2wt section of phpconfig.yaml
* The single failing html2html test is a <gallery> test which is
presumably related to the unported <gallery> extension code, but
not sure. Not investigating it now.
* Update Parsoid Extension API to provide access to extension source
without exposing internals.
Change-Id: I6d6e21ad2324acfc4306b32c9055d6c088708c48
- git grep is a wonderful thing to help catch identical errors.
- Saw this "bug" first in the cite extension port.
- Turns out this is only a bug on the PHP side since the PHP DOM
treats attributes in a case-sensitive manner but Domino.js
treats attributes in a case-insensitive manner.
- Better to use the correct attribute name everywhere.
Change-Id: I3735dc768a10a820b4816c211aa72291df9b1413
* unwrapFragment had a somewhat unusual behavior which could be
a source of bugs while reasoning with it.
If undefined, it's default value is true which is contrary
to how we think of undefined.
* Flipping the polarity of the flag to sealFragment makes the
semantics easier to reason with and where !empty(..) applies
more naturally to it.
Change-Id: Ia50cba345f37e815e5f5f95abb452c8eefcf9011
In PHP, DOMNode#getAttribute() return '' if the attribute is not present,
not null. Audit our uses and try to either explicitly use `|| ''` (which
will ensure that PHP behaves the same was as JS) or use `hasAttribute`
to explicitly test for the presence of the attribute.
Changes have also been ported to PHP from JS.
Also added src/Wt2Html/PP/Processors/AddMediaInfo.php which was missing.
Change-Id: Ie1ae1df88e4fca70daf97b6f720f28014ebc99ed
This applies the JS changes from the following recently-merged patches:
6679c3bf Protect data-object-id attribute
d4e76d5b Fix new linter category to enable code work with templates
e567db8d Tweak storeDataAttribs to suppress DOM nodes in data-parsoid.tmp
16603953 Fix setting dsr on body for genTest
3a84a9dd Fix stashing data attributes for mw:StartTag
22c4a19a Remove redundant dataParsoid call
ed7b0ba0 Fix crasher in newly added linter category
505a357b Linter.js: Add new function to detect the use of links in links
8885b20e Move redlink updating into lib/parse.js
ccfce23d templatedepth is either an int or false
6d1571bd Move language conversion work into lib/parse.js
5a89c7de Avoid serialize/parse of data attributes when treebuilding
021d9958 Rename `document.env` to `document.bag`
c03ba494 Use XMLSerializer on both PHP & JS side in the DOM pass test script
e0c3cca9 Use env.createDocument in lib/api/apiUtils.js
550d3d71 Use a bag-on-the-side implementation for node data
f8de8b25 Add bin/inspectTokenizer.js
db704eea Add ability to splice a PHP transformer into the pipeline
a8be3ad6 Fix crasher in cite extension from accessing data after it's stored
2874f200 Simplify and clean up stops usage
6368265d Add some strategic isElt guards
5ae9553f DRY out transform test runners + tweak genTest to enable that
b0f2adc6 Assert that the .dataobject isn't touched after storing attrs on a node
1ce6a98d Skip separators when looking for the next th/td
Change-Id: I6a66ecb061e7ee7ed53feba1895dd315d9324715
The basic idea here is to generate the media structure in the token
stream using a stuffed span with a redlink, as in T169975, and
augmenting the nodes on the DOM once the media info has been fetched.
A redlink is justified as the canonical representation of the media
elements before info is fetched because it's the fallback if fetching
fails and the media type is unknown until the info is retrieved.
Most options are stored in data-mw until the media type is fetched and
it's determined that they're applicable. This is a bit of a reversion
of how things were done before where inapplicable options were removed
post-facto.
For consistency and styling's sake, figcaptions are now always added to
block figures.
The pass has to be run before generating headings anchor, since that
depends on the text content (ie. redlinks). This rearranges things in
the post-processor and adds another pass.
The post-processing pass to add media info is run on subpipelines as
well as the top level so that the media info is present in cases where
we embed HTML in data-mw (which is currently skipped by the top level
only passes, except for the cite extension, which has special handling,
see T214994) and to avoid an additional post-processing pass for the
gallery extension, which scales media of packed galleries. This comes
at the cost of making additional queries for each pipeline and requires
the add media pass to be idempotent.
Filed T214241 for figuring out what to do about data-mw info being
clobbered by template annotations.
The newly failing blacklisted tests are from roundtripping media options
in galleries, which requires a general refactor for support. See the
FIXMEs added there.
Performance should be expected to regress by the amount of work we're
able to overlap in the async phase of the pipeline while the media info
is being fetched. Considering a lot of that work is caught up waiting
for the batch to return (other async requests are found in the same
batch), this doesn't turn out to be much in practice in the average
case.
Bug: T153080
Bug: T169975
Change-Id: I856ee962b70cef1f8d49652396ea5264e11a8ade
* This initialization lets us do a git log --follow and follow
git history for that file across the js -> php port boundary.
This works because git uses content hashes for objects and
the copied code in the new .php file will have the same content
hash as the .js file.
* The following directories were skipped
- ./lib/config/baseconfig
- ./tests
* The following JS files were skipped
- ./lib/utils/promise.js
- ./lib/config/wmf.sitematrix.json
- ./tools/sync-baseconfig.js
- ./tools/sync-parserTests.js
- ./tools/fetch_ve_nowiki_edits.js
- ./tools/fetch-parserTests.txt.js
- ./tools/fetch-wmf-sitematrix.js
- ./tools/compare.linter.results.js
- ./tools/fetch-revision-data.js
- ./tools/fetch-wt.js
- ./tools/regression-testing.js
- ./tools/build-langconv-fst.js
- ./bin/server.js
Change-Id: I0b22057c23b72795aebbd66e3abcb627c6858ef3
* Extensions only need config info (env) and potentially abstract
parsing context (frame).
* Added some FIXMEs about potential future improvements.
Change-Id: Ib4cec4a77ecb96c855798eeb06f7742c5efb0729
It's only the extension content that needs the about ids. This matches
b241ac6 where span wrapping was only applied to extension content, and
goes hand-in-hand. The one necessitates the other.
Fragment wrappers don't need abouts since they don't have siblings and
are represented as a single node.
When we get to unpacking, it's now clear that if the wrapper has an
about id, it came from template encapsulation and would need to be
applied to the top-level nodes in the fragment. This being true of
all fragment, it lets use get rid of the `isForeignContent` option to
`encapsulateExpansionHTML`, since that no longer serves to distinguish
anything.
Change-Id: Id9faae3d4e3c8771c2de1fb42ba62a7d92d76673
