[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[matej_suchanek]

matej_suchanek closed this task as "Resolved".
matej_suchanek claimed this task.

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: matej_suchanek
Cc: Michael, Lucas_Werkmeister_WMDE, 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 -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[ReleaseTaggerBot]

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.23; 2023-02-13).

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: ReleaseTaggerBot
Cc: Michael, Lucas_Werkmeister_WMDE, 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 -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[Maintenance_bot]

Maintenance_bot removed a project: Patch-For-Review.

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Maintenance_bot
Cc: Michael, Lucas_Werkmeister_WMDE, 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, Themindcoder, Adamm71, 
Jersione, Hellket777, LisafBia6531, 786, Biggs657, Juan90264, Alter-paule, 
Beast1978, Un1tY, Hook696, Kent7301, joker88john, CucyNoiD, Gaboe420, 
Giuliamocci, Cpaulf30, Af420, Bsandipan, Lewizho99, Maathavan, Neuronton
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[gerritbot]

gerritbot added a comment.


  Change 620381 **merged** by jenkins-bot:
  
  [mediawiki/extensions/Wikibase@master] Construct parsers with a copy of 
ParserOptions
  
  https://gerrit.wikimedia.org/r/620381

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: gerritbot
Cc: Michael, Lucas_Werkmeister_WMDE, matej_suchanek, Aklapper, Themindcoder, 
Adamm71, Jersione, Hellket777, LisafBia6531, Astuthiodit_1, 786, Biggs657, 
karapayneWMDE, Invadibot, maantietaja, Juan90264, Alter-paule, Beast1978, 
ItamarWMDE, Un1tY, Akuckartz, Hook696, Kent7301, joker88john, CucyNoiD, 
Nandana, Gaboe420, lucamauri, Giuliamocci, Cpaulf30, Lahi, Gq86, Af420, 
Bsandipan, GoranSMilovanovic, QZanden, LawExplorer, Lewizho99, Maathavan, 
_jensen, rosalieper, Neuronton, Scott_WUaS, Wikidata-bugs, aude, Mbch331
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[matej_suchanek]

matej_suchanek added a comment.


  > It's still quite a mystery to me why that happens and why it happens for 
Czech, but not e.g., English. (I suspect the results come from 
PhpDateTimeParser.)
  
  Not a mystery anymore: T325988 .

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: matej_suchanek
Cc: Lucas_Werkmeister_WMDE, matej_suchanek, Aklapper, Adamm71, Jersione, 
Hellket777, LisafBia6531, Astuthiodit_1, 786, Biggs657, karapayneWMDE, 
Invadibot, maantietaja, Juan90264, Alter-paule, Beast1978, ItamarWMDE, Un1tY, 
Akuckartz, Hook696, Kent7301, joker88john, CucyNoiD, Nandana, Gaboe420, 
lucamauri, Giuliamocci, Cpaulf30, Lahi, Gq86, Af420, Bsandipan, 
GoranSMilovanovic, QZanden, LawExplorer, Lewizho99, Maathavan, _jensen, 
rosalieper, Neuronton, Scott_WUaS, Wikidata-bugs, aude, Mbch331
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[matej_suchanek]

matej_suchanek added a comment.


  I was primarily focused on fixing the parent task T221097 
. When you use Czech (cs) as the 
interface language and type some dates in the format "DD.MM." or "DD. MM. 
", they are either parsed with the day and month swapped or the produced 
date is completely nonsensical. (This does not happen when you use English.)
  For example:
  
  - "11. 12. 2022" is interpreted 

 as "November 12th, 2022", although "December 11th, 2022" is expected.
  - "07.05.1997" is interpreted 

 as "June 30th, 1997" (huh?), although "May 7th, 1997" is expected.
  
  It's still quite a mystery to me why that happens and why it happens for 
Czech, but not e.g., English. (I suspect the results come from 
`PhpDateTimeParser`.)
  The patch (despite looking like a bugfix) effectively adds (or activates) a 
parser that parses dates of "date" type. The format of these dates depends on 
the input language:
  
> \Language::factory('cs')->getDateFormatString('date', 'default')
= "j. n. Y"

> \Language::factory('de')->getDateFormatString('date', 'default')
= "j. F Y"

> \Language::factory('en')->getDateFormatString('date', 'default')
= "j F Y"

> \Language::factory('ru')->getDateFormatString('date', 'default')
= "j xg Y"
  
  Provided that the `DateFormatParser` works as intended, the "new" parser 
should now catch these dates and parse them correctly.
  I hope this interpretation will help evaluate the actual impact.

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: matej_suchanek
Cc: Lucas_Werkmeister_WMDE, matej_suchanek, Aklapper, Adamm71, Jersione, 
Hellket777, LisafBia6531, Astuthiodit_1, 786, Biggs657, karapayneWMDE, 
Invadibot, maantietaja, Juan90264, Alter-paule, Beast1978, ItamarWMDE, Un1tY, 
Akuckartz, Hook696, Kent7301, joker88john, CucyNoiD, Nandana, Gaboe420, 
lucamauri, Giuliamocci, Cpaulf30, Lahi, Gq86, Af420, Bsandipan, 
GoranSMilovanovic, QZanden, LawExplorer, Lewizho99, Maathavan, _jensen, 
rosalieper, Neuronton, Scott_WUaS, Wikidata-bugs, aude, Mbch331
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[Lucas_Werkmeister_WMDE]

Lucas_Werkmeister_WMDE added a comment.


  I agree that the code is broken, but what is the actual impact of this? So 
far I haven’t managed to find an example where the parsing is broken due to 
this – do you have one?

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: Lucas_Werkmeister_WMDE
Cc: Lucas_Werkmeister_WMDE, matej_suchanek, Aklapper, Adamm71, Jersione, 
Hellket777, LisafBia6531, Astuthiodit_1, 786, Biggs657, karapayneWMDE, 
Invadibot, maantietaja, Juan90264, Alter-paule, Beast1978, ItamarWMDE, Un1tY, 
Akuckartz, Hook696, Kent7301, joker88john, CucyNoiD, Nandana, Gaboe420, 
lucamauri, Giuliamocci, Cpaulf30, Lahi, Gq86, Af420, Bsandipan, 
GoranSMilovanovic, QZanden, LawExplorer, Lewizho99, Maathavan, _jensen, 
rosalieper, Neuronton, Scott_WUaS, Wikidata-bugs, aude, Mbch331
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[gerritbot]

gerritbot added a project: Patch-For-Review.

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: gerritbot
Cc: matej_suchanek, Aklapper, Adamm71, Jersione, Hellket777, LisafBia6531, 
Astuthiodit_1, 786, Biggs657, karapayneWMDE, Invadibot, maantietaja, Juan90264, 
Alter-paule, Beast1978, ItamarWMDE, Un1tY, Akuckartz, Hook696, Kent7301, 
joker88john, CucyNoiD, Nandana, Gaboe420, lucamauri, Giuliamocci, Cpaulf30, 
Lahi, Gq86, Af420, Bsandipan, GoranSMilovanovic, QZanden, LawExplorer, 
Lewizho99, Maathavan, _jensen, rosalieper, Neuronton, Scott_WUaS, 
Wikidata-bugs, aude, Mbch331
___
Wikidata-bugs mailing list -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[gerritbot]

gerritbot added a comment.


  Change 620381 had a related patch set uploaded (by Matěj Suchánek; author: 
Matěj Suchánek):
  
  [mediawiki/extensions/Wikibase@master] Construct parsers with a copy of 
ParserOptions
  
  https://gerrit.wikimedia.org/r/620381

TASK DETAIL
  https://phabricator.wikimedia.org/T324392

EMAIL PREFERENCES
  https://phabricator.wikimedia.org/settings/panel/emailpreferences/

To: gerritbot
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 -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org

[Wikidata-bugs] [Maniphest] T324392: Fix unintentional side-effect of MwDateFormatParserFactory

[matej_suchanek]

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 
.
  
  An obvious solution is cloning the ParserOptions instance and passing a 
(shallow) copy (change 
).
 But then, as previously dead code is now active, some tests start to fail, 
making the change non-trivial (patch for review 
).
  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 -- wikidata-bugs@lists.wikimedia.org
To unsubscribe send an email to wikidata-bugs-le...@lists.wikimedia.org