Commit graph

3352 commits

Author SHA1 Message Date
jenkins-bot ce14db4048 Merge "Add parser tests for the responsive="…" feature" 2019-12-18 07:10:22 +00:00
Thiemo Kreuz 1f76199ed8 Add parser tests for the responsive="…" feature
Change-Id: Id9d733dabf82f2c26f51c6fbd1e03fe0574e88a8
2019-12-17 15:51:41 +01:00
Thiemo Kreuz e5cda65fbe Remove single use classes from the use section
The name of the base class in tests is guaranteed to only occur a
single time in a file. There is not much value in making it relative,
and requiring it to appear in the use section. Especially because it
is in the root namespace.

This reflects what I once encoded in the sniff
https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/Sniffs/Namespaces/FullQualifiedClassNameSniff.php
I wish we could pick this rule and use it in our codebases. But it
seems it is to specific and can't be applied on all codebases, hence
it can't become part of the upstream MediaWiki rule set. At least not
at the moment.

Change-Id: I77c2490c565b7a468c5c944301fc684d20206ec4
2019-12-17 14:57:55 +01:00
Thiemo Kreuz 1bd66081f7 Fix inconsistent error reporting for invisible content
This makes one of the last remaining edge-cases about non-empty, but
non-visible content (a <ref> that only contains whitespace) behave
identical to all other places. We already reported it as being empty
everywhere else, except inside of <references>.

Note that the test cases look like they are reporting the same errors
twice. But this is not the case:

The first set of errors is about <ref name="…"> inside of <references>
not having visible content. This should always be reported, even if the
<ref> got content from somewhere else on the page.

The second set of errors is when a <ref name="…"> *never* got any
content.

This patch will slightly increase the numbers of errors reported.

Change-Id: I4a156aa9e466f735d92fe0ba5cc0678ec8bbdd50
2019-12-17 13:37:01 +01:00
Translation updater bot 50c559c7d8 Localisation updates from https://translatewiki.net.
Change-Id: I1fbcf34a10685b30b142fd4d08fd5f22ae3e43d5
2019-12-17 10:06:57 +01:00
libraryupgrader 55720346c8 build: Updating mediawiki/minus-x to 0.3.2
Change-Id: I6abda0708cc29b0ca74950e6022cc81a032a0a61
2019-12-16 21:16:23 +00:00
Thiemo Kreuz 51ff3cc819 Several code cleanups after getting rid of cloning
* Use the Html class to safely create HTML code.
* $this->referenceStack can not be null any more.
* $this->inReferencesGroup is not needed during output, only when
  parsing tags.
* Replace ReferencesStack::getGroupRefs() as well as deleteGroup()
  with a combined popGroup() that does both things.
* Extract the code responsible for the "responsive" behavior to a
  separate function.
* Some TestingAccessWrapper are not needed.

Change-Id: Ie1cf2533d7417ae2f6647664ff1145e37b814a39
2019-12-16 15:47:23 +01:00
jenkins-bot b9e6bf2340 Merge "Simplify null comparison" 2019-12-16 14:01:31 +00:00
jenkins-bot fad482c418 Merge "Rename test to match class" 2019-12-16 14:01:30 +00:00
jenkins-bot 1a76a1d6e4 Merge "Remove Parser state from CiteErrorReporter" 2019-12-16 14:01:28 +00:00
jenkins-bot 277d3e84da Merge "Remove Parser state from ReferencesFormatter" 2019-12-16 12:47:57 +00:00
jenkins-bot 67aa6b22e5 Merge "Remove Parser state from FootnoteMarkFormatter" 2019-12-16 12:47:55 +00:00
jenkins-bot 3028651ebf Merge "Don't keep parser reference in Cite" 2019-12-16 12:47:53 +00:00
jenkins-bot 3640605a1e Merge "Lazy instantiation of Cite" 2019-12-16 12:47:52 +00:00
Translation updater bot aae8297f0f Localisation updates from https://translatewiki.net.
Change-Id: Ic8c6bd20bda69a653ee66c91b73119b8be13da68
2019-12-13 09:27:39 +01:00
Adam Wight 9fd825e6a6 Simplify null comparison
In these cases, an expression is either true-ish or null, so we can use
the implicit boolean cast to test.

Change-Id: Ibe94829f9774bf2a1907635a8bd28369908b4d1e
2019-12-12 16:42:53 +01:00
Adam Wight 870b9ec181 Rename test to match class
Change-Id: I1814f31ead3882d4c484e0da2808fc7fbc9d2f37
2019-12-12 11:15:13 +01:00
Adam Wight ab07a3253c Remove Parser state from CiteErrorReporter
Finishes breaking the circular reference between Cite and Parser.

This patch also demonstrates how evil it is to allow the error reporter
to be called from anywhere, and have side-effects.  At least it's explicit
now.

Also fixes a bug where the inner error message would not be in the
interface language.

Bug: T240431
Change-Id: Ic3325cafb503e78295d72231ac6da5c121402def
2019-12-12 11:15:07 +01:00
Adam Wight 22b2f78db6 Remove Parser state from ReferencesFormatter
Bug: T240431
Change-Id: I3477a64b9a9dba0cfe890c4b598a51c2f971c76c
2019-12-12 11:12:17 +01:00
Adam Wight 2a5976f007 Remove Parser state from FootnoteMarkFormatter
Bug: T240431
Change-Id: Ie53444114c032e083293d3b5325252debb0640a7
2019-12-12 11:12:17 +01:00
Adam Wight 852a503262 Don't keep parser reference in Cite
This begins our journey of breaking the circular reference between
Cite and Parser.  In later patches the child objects will also take
Parser as a parameter.

Bug: T240431
Change-Id: Ic672bb4bae19ac5f1e1f5817de171d76b3bd8786
2019-12-12 11:12:17 +01:00
Adam Wight a227395e3a Lazy instantiation of Cite
Only create a Cite object if we need one.  Never clearState, just
destroy and recreate later.

This makes it less likely that we leak state between parsers, and
saves memory and processing on pages without references.

It's also preparation to decouple Cite logic from state.

Change-Id: I3db517591f4131c23151c76c223af7419cc00ae9
2019-12-12 11:12:17 +01:00
Translation updater bot ef98c9826d Localisation updates from https://translatewiki.net.
Change-Id: Id2e702414c89a42bc04f6f080fd08443da15ff94
2019-12-12 09:56:19 +01:00
Thiemo Kreuz 3f2aeb7e31 Rename two Cite… classes and clean up test setups
* All classes are in a Cite\ namespace now. No need to repeat the word
"Cite" all over the place.

* The "key formatter" is more an ID or anchor formatter. The strings it
returns are all used in id="…" attributes, as well as in href="#…" links
to jump to these IDs.

* This patch also removes quite a bunch of callbacks from tests that
don't need to be callbacks.

* I'm also replacing all json_encode().

* To make the test code more readable, I shorten a bunch of variable
names to e.g. $msg. The fact they are mocks is still relevant, and still
visible because these variable names are only used in very short scopes.

Change-Id: I2bd7c731efd815bcdc5d33bccb0c8e280d55bd06
2019-12-12 08:48:02 +01:00
jenkins-bot 69a8754e31 Merge "Rename "index" parameter to "key"" 2019-12-11 16:25:30 +00:00
jenkins-bot 97e144755d Merge "Rename "key" variable to "lookup"" 2019-12-11 16:25:29 +00:00
jenkins-bot 9cc9e9cdc1 Merge "Add parser tests for reused extended <ref> before defined" 2019-12-11 16:04:53 +00:00
jenkins-bot 5b7f6d2d35 Merge "Add parser test for duplicate extended references" 2019-12-11 15:35:33 +00:00
jenkins-bot 0dafe64305 Merge "Rename CiteParserTagHooks::initialize to register" 2019-12-11 15:35:32 +00:00
jenkins-bot 5d5fd30a3c Merge "Minor improvements to the test coverage" 2019-12-11 15:35:32 +00:00
Thiemo Kreuz f86b5073fd Add parser tests for reused extended <ref> before defined
Bug: T240424
Change-Id: I945c2e12cfa3ff851380a1ff4491c8af076f523a
2019-12-11 16:30:17 +01:00
Thiemo Kreuz 193b840010 Add parser test for duplicate extended references
Bug: T240459
Change-Id: Ifc7a695e89a49ccc6c66d49efe41b2321b0915f0
2019-12-11 15:58:34 +01:00
jenkins-bot f642669522 Merge "Rename $type to $action in rollbackRef()" 2019-12-11 11:49:25 +00:00
jenkins-bot e39b1d6cbd Merge "Use messagelocalizer in CiteErrorReporter" 2019-12-11 11:42:28 +00:00
Thiemo Kreuz d0cb639e03 Minor improvements to the test coverage
We are *so* close to 90%.

This patch should raise the coverage for the CiteDataModule to 100%.
I'm also adding a pure unit test for the clone() behavior. Note the
later is already covered by the CiteDbTest.

Question: Do we want the CiteDbTest to @cover anything?

Change-Id: I40763d01e18991f509bc30b6655aa57b23412fd9
2019-12-11 12:22:59 +01:00
Adam Wight f93f1b4fe0 Use messagelocalizer in CiteErrorReporter
Fixes a bug introduced in Icf61c9a27fd, which would cause a parser
cache split any time the Cite extension was initialized.  The
`setLanguage` interface is regrettable, but I'm hoping it will only
be around temporarily.

Converts an integration test into a unit test and completes coverage.

Bug: T239988
Change-Id: I4b1f8909700845c9fa0cbc1a3de50ee7d42f69a5
2019-12-11 09:53:47 +01:00
jenkins-bot 6556f75c38 Merge "Integration parser test for ParserFunctions" 2019-12-11 08:45:34 +00:00
jenkins-bot 76b4706938 Merge "Rename formatNumNoSeparators() to localizeDigits()" 2019-12-11 08:29:09 +00:00
jenkins-bot 37ea791a4c Merge "Use a guard clause in Cite::checkRefsNoReferences" 2019-12-11 08:18:18 +00:00
Adam Wight 084ca6b3d4 Integration parser test for ParserFunctions
Depends-On: I09844079f163e583d3b1e941c701f8cda5029a0a
Bug: T240345
Change-Id: I86c55ff88d9f4b800e8868728dfec6b4ceda82c2
2019-12-11 08:58:58 +01:00
Adam Wight cad4d18458 Integration test to hit cloned Cite bug
Tickle very particular edge case in which a recursive parse corrupts
the $parser->extCite object.

Bug: T240248
Change-Id: I70d100e88fa72825194ed9c477b030bbf0b6b486
2019-12-10 17:07:44 +01:00
Thiemo Kreuz 66069d9dcf Use a guard clause in Cite::checkRefsNoReferences
This patch also cleans up a few pieces of PHPDoc documentation.

Change-Id: Ib207b11121769c543723db4668786f4916470368
2019-12-10 15:33:53 +00:00
Thiemo Kreuz 75016551e7 Rename formatNumNoSeparators() to localizeDigits()
Because that is what it does. Note our method is different from the one
in the Language class. We only accept strings.

Change-Id: I39107e837cc29f2d7c8867c1e602aa643f9e1a57
2019-12-10 16:21:12 +01:00
Thiemo Kreuz 83fc992159 Rename $type to $action in rollbackRef()
We call this an action when creating elements on the refCallStack.

Change-Id: I50e9df2f396060623e7e6b6deda086783709712b
2019-12-10 16:04:59 +01:00
Thiemo Kreuz df1a45b84c Fix incomplete cloning of the Parser::$extCite instance
This is what happens:
* The issue happens only on pages with two <ref> tags than share the
same name and group, but have conflicting text.
* This triggers a code path that renders an error message and calls
Message::plain() as well as Parser::addTrackingCategory(), which calls
Message::text().
* The Message class is asking for a new, fresh parser. This means the
parser is cloned and it's state cleared, while keeping stuff like
parser hooks.
* Cloning the parser triggers the ParserCloned hook.
* The hook handler clones the Cite instance stored in Parser::$extCite.
* PHP doesn't do deep cloning. Object properties are not cloned.
* Since I091a0b7 the internal state of the Cite class is extracted to
another class.
* This means the state is not cloned any more since I091a0b7.
* Now two Cite instances share the same state.
* At the end of the hook handler, the state is cleared, which also
clears the state of the original instance.

We will most probably solve this on master by getting rid of cloning
Cite. We propose this additional hotfix for the branch.

Bug: T240248
Change-Id: Ic5a438e04d003a637ae08aae936d9977cc90d5d3
2019-12-10 14:20:00 +00:00
Thiemo Kreuz 01bcfa773d Rename CiteParserTagHooks::initialize to register
It's called "register" in MediaWiki core as well.

Change-Id: Iad3dc3badbb7ad10a14276c3a144376acf70e5e5
2019-12-10 14:19:33 +00:00
jenkins-bot df5068094e Merge "Localisation updates from https://translatewiki.net." 2019-12-10 08:32:58 +00:00
Translation updater bot 647640f1a4 Localisation updates from https://translatewiki.net.
Change-Id: I917901bc6de019082c5567b6296d603481d6f679
2019-12-10 09:29:05 +01:00
Thiemo Kreuz 71c6dc7fe4 Better naming for ReferenceFormatter class and methods
This class renders a <references> tag and everything inside. The
previous name sounds like it is responsible for rendering the contents
of a <ref>…</ref> tag. I mean, the class contains a method that does
exactly this. But this method is private.

Change-Id: I1cd06c9a11e0a74104f2874a34efa3e0843a0f70
2019-12-10 08:40:09 +01:00
jenkins-bot f6990712d3 Merge "Fix config for some book referencing parser tests" 2019-12-09 18:24:17 +00:00