[Wikidata-bugs] [Maniphest] [Commented On] T231084: Assert.php: Bad value for parameter $oldContent: must be a TextContent|null [Story Points 5]

2020-02-04 Thread Tgr 
Tgr added a comment.


  In T231084#5839574 , 
@Addshore wrote:
  
  > With canDiffRevisions being implemented in DifferenceEngine which would see 
if the 2 contents say they can be diffed with each other?
  
  How would DifferenceEngine know that? It has the same dependency on page type 
as the slots.
  
  > I mean, when would it ever make sense to even try and diff wikitext and a 
wikibase entity?
  
  It probably doesn't. It would still be nice if Wikibase could make that call.
  
  Anyway the immediate solution is to just throw a UserPageError instead of the 
assertion error, and have it tell the user that incompatible revisions are 
being compared (and maybe tell the content models).
  
  In the longer term, what if you compare a (wikitext, foo) MCR record with a 
(wikibase, foo) one? Does it still make sense to at least the foo part diffed? 
If so, DifferenceEngine would probably need its own error handling, which turns 
the exception into part of the diff HTML.

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

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

To: Tgr
Cc: Addshore, Umherirrender, Ladsgroup, Tgr, Michael, Lucas_Werkmeister_WMDE, 
Aklapper, darthmon_wmde, Nandana, Lahi, Gq86, GoranSMilovanovic, QZanden, 
LawExplorer, _jensen, rosalieper, Scott_WUaS, Jonas, Wikidata-bugs, aude, 
Lydia_Pintscher, Jdforrester-WMF, Mbch331, Rxy, Jay8g, Krenair
___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T231084: Assert.php: Bad value for parameter $oldContent: must be a TextContent|null [Story Points 5]

2020-01-30 Thread Addshore 
Addshore added a comment.


  Another option would be something like:
  
$de = $contentHandler->createDifferenceEngine( 
$form->getContext(),
$rev1,
$rev2,
null, // rcid
( $data['Action'] == 'purge' ),
( $data['Unhide'] == '1' )
);
if(!$de->canDiffRevisions();){
// Throw special page error here
}
$de->showDiffPage( true );
  
  With canDiffRevisions being implemented in DifferenceEngine which would see 
if the 2 contents say they can be diffed with each other?
  I mean, when would it ever make sense to even try and diff wikitext and a 
wikibase entity?

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

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

To: Addshore
Cc: Addshore, Umherirrender, Ladsgroup, Tgr, Michael, Lucas_Werkmeister_WMDE, 
Aklapper, darthmon_wmde, Nandana, Lahi, Gq86, GoranSMilovanovic, QZanden, 
LawExplorer, _jensen, rosalieper, Scott_WUaS, Jonas, Wikidata-bugs, aude, 
Lydia_Pintscher, Jdforrester-WMF, Mbch331, Rxy, Jay8g, Krenair
___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs

[Wikidata-bugs] [Maniphest] [Commented On] T231084: Assert.php: Bad value for parameter $oldContent: must be a TextContent|null [Story Points 5]

2019-12-16 Thread Ladsgroup 
Ladsgroup added a comment.


  In T231084#5444984 , 
@Lucas_Werkmeister_WMDE wrote:
  
  > On the other hand, other content models sometimes seem to support this – 
this diff 
,
 for example, is between a wikitext page and a JSON page. (It also works if you 
swap the compared revisions, for what it’s worth.)
  
  It's because JsonContent extends TextContent thus it passes the check while 
EntityContent doesn't (we can also make EntityContent extend it but it seems a 
little weird to me)
  
  In T231084#5569517 , 
@Umherirrender wrote:
  
  > Special:ComparePages makes it better 
https://www.wikidata.org/wiki/Special:ComparePages?page1==11==772301402===
  >
  >> Can not diff an entity with a non-entity content.
  >
  > wikibase-non-entity-diff
  > But not the other way round: 
https://www.wikidata.org/wiki/Special:ComparePages?page1==772301402==11===
  >
  >> [XaGZeQpAIC8AAHHLlKwI] 2019-10-12 09:14:33: Fatal 
„Wikimedia\Assert\ParameterTypeException“
  >
  > So the question seems which diffengine is instanced for the compare and 
what for checks it does
  
  The traceback for this fatal doesn't hit any part of Wikibase code, making it 
hard to fix:
  
Backtrace:

#0 /var/lib/mediawiki2/includes/diff/SlotDiffRenderer.php(86): 
Wikimedia\Assert\Assert::parameterType(string, Wikibase\ItemContent, string)
#1 /var/lib/mediawiki2/includes/diff/TextSlotDiffRenderer.php(119): 
SlotDiffRenderer->normalizeContents(Wikibase\ItemContent, WikitextContent, 
string)
#2 /var/lib/mediawiki2/includes/diff/DifferenceEngine.php(1137): 
TextSlotDiffRenderer->getDiff(Wikibase\ItemContent, WikitextContent)
#3 /var/lib/mediawiki2/includes/diff/DifferenceEngine.php(1055): 
DifferenceEngine->getDiffBody()
#4 /var/lib/mediawiki2/includes/diff/DifferenceEngine.php(1017): 
DifferenceEngine->getDiff(string, string, string)
#5 /var/lib/mediawiki2/includes/diff/DifferenceEngine.php(781): 
DifferenceEngine->showDiff(string, string, string)
#6 /var/lib/mediawiki2/includes/specials/SpecialComparePages.php(128): 
DifferenceEngine->showDiffPage(boolean)
#7 /var/lib/mediawiki2/includes/htmlform/HTMLForm.php(694): 
SpecialComparePages::showDiff(array, OOUIHTMLForm)
#8 /var/lib/mediawiki2/includes/specials/SpecialComparePages.php(109): 
HTMLForm->trySubmit()
#9 /var/lib/mediawiki2/includes/specialpage/SpecialPage.php(575): 
SpecialComparePages->execute(NULL)
#10 /var/lib/mediawiki2/includes/specialpage/SpecialPageFactory.php(611): 
SpecialPage->run(NULL)
#11 /var/lib/mediawiki2/includes/MediaWiki.php(298): 
MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#12 /var/lib/mediawiki2/includes/MediaWiki.php(967): 
MediaWiki->performRequest()
#13 /var/lib/mediawiki2/includes/MediaWiki.php(530): MediaWiki->main()
#14 /var/lib/mediawiki2/index.php(46): MediaWiki->run()
#15 {main}
  
  The main one is not super hard to fix though. I can put a try catch in 
ViewEntityAction::showEntityPage():
  
try {
$this->page->view();
} catch ( ParameterTypeException $exception ) {
if ( !$this->isDiff() === true ) {
throw $exception;
}
}
  
  Not sure this is the best way to move forward. One other rather easy way is 
to have the try catch in DifferenceEngine::showDiffPage.

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

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

To: Ladsgroup
Cc: Umherirrender, Ladsgroup, Tgr, Michael, Lucas_Werkmeister_WMDE, Aklapper, 
darthmon_wmde, Nandana, Lahi, Gq86, GoranSMilovanovic, QZanden, LawExplorer, 
_jensen, rosalieper, Scott_WUaS, Jonas, Wikidata-bugs, aude, Lydia_Pintscher, 
Jdforrester-WMF, Mbch331, Rxy, Jay8g, Krenair
___
Wikidata-bugs mailing list
Wikidata-bugs@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikidata-bugs