matej_suchanek created this task.
matej_suchanek added projects: Wikidata,
MediaWiki-extensions-WikibaseRepository.
Restricted Application added a subscriber: Aklapper.
TASK DESCRIPTION
name=TimeParserFactory::getTimeParsers
$mwDateFormatParserFactory = new MwDateFormatParserFactory();
$parsers[] = $mwDateFormatParserFactory->getMwDateFormatParser(
$this->options->getOption( ValueParser::OPT_LANG ),
$dateFormatPreference,
'date', // <-- $dateFormatType
$this->options
);
$parsers[] = $mwDateFormatParserFactory->getMwDateFormatParser(
$this->options->getOption( ValueParser::OPT_LANG ),
$dateFormatPreference,
'monthonly', // <-- $dateFormatType
$this->options
);
Notice that both calls receive the same instance of ParserOptions, which is
passed by reference.
name=MwDateFormatParserFactory::getMwDateFormatParser
$dateFormat = $language->getDateFormatString( $dateFormatType,
$dateFormatPreference );
// [...]
if ( $options === null ) {
$options = new ParserOptions();
}
$options->setOption( DateFormatParser::OPT_DATE_FORMAT, $dateFormat );
// [...]
return new DateFormatParser( $options );
On the first pass, `DateFormatParser::OPT_DATE_FORMAT` is set with the
`$dateFormat` returned from `Language::getDateFormatString`, which depends on
the `$dateFormatType` (i.e., `'date'` on the first pass).
On the second pass, `DateFormatParser::OPT_DATE_FORMAT` is set with the
`$dateFormat` returned from `Language::getDateFormatString`, which depends on
the `$dateFormatType` (i.e., `'monthonly'` on the first pass).
However, this is done on the **same instance** of ParserOptions, which both
new `DateFormatParser`'s share. The second pass therefore overwrites the first
pass, and both `DateFormatParser`'s are provided with the same value for
`DateFormatParser::OPT_DATE_FORMAT`, making one of them basically useless. This
was probably broken in rEWBA61c58c1e2e92: Make new (Mw)DateFormatParser respect
user-provided precision
<https://phabricator.wikimedia.org/rEWBA61c58c1e2e924c5a2fc27ec5cdb56ee34f346472>.
An obvious solution is cloning the ParserOptions instance and passing a
(shallow) copy (change
<https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/620381/5/repo/includes/Parsers/TimeParserFactory.php>).
But then, as previously dead code is now active, some tests start to fail,
making the change non-trivial (patch for review
<https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/620381/5>).
Also, `TimeParserFactory::getTimeParsers` constructs more than just the two
parsers, which all share the same ParserOptions instance. Maybe this kind of
bug affects other parsers, too.
TASK DETAIL
https://phabricator.wikimedia.org/T324392
EMAIL PREFERENCES
https://phabricator.wikimedia.org/settings/panel/emailpreferences/
To: matej_suchanek
Cc: matej_suchanek, Aklapper, Astuthiodit_1, karapayneWMDE, Invadibot,
maantietaja, ItamarWMDE, Akuckartz, Nandana, lucamauri, Lahi, Gq86,
GoranSMilovanovic, QZanden, LawExplorer, _jensen, rosalieper, Scott_WUaS,
Wikidata-bugs, aude, Mbch331
_______________________________________________
Wikidata-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
