Autoloader changes on trunk, new readability implementation


#1
  1. Readability library changed to a different implementation: https://github.com/andreskrey/readability.php
  2. Class autoloader has been updated to support the above, incidentally we can now load namespaced third party libraries with zero additional bullshit (should’ve done this long time ago tbh)

Report any issues here.


#2

Just tried the new readability library. It seems to work a lot better than the other one. This even removes a good amount of the garbage sites put in the middle of their articles (related stories, etc.).


#3

glad to hear it, i only have two sites with readability enabled and i haven’t noticed any difference :slight_smile:

previous library was abandonware so at this point anything would be an improvement


#4

Can’t log in after update. First, Chrome throws HTTP Error 500, then, on reload:

{"error":{"code":13,"message":"Method not found"}}

Old TT-RSS: d00d515
VPS, Debian 9, php 7.0.27, nginx 1.10.3, PostgreSQL 9.6.7.


#5

check nginx error.log for errors, also try going to older changesets until things start working again and then check tt-rss error log in preferences for errors.

anything non-stock in config.php like auth modules?

e: nvm, got it replicated

this should fix it


#6

It has. Thanks! Works again.


#7

Issue: feed using readability to inline content causes the following error to be recorded.

PHP Parse error:  syntax error, unexpected 'class' (T_CLASS), expecting identifier (T_STRING) or variable (T_VARIABLE) or '{' or '$' in /home/feeds/ttrss/vendor/andreskrey/Readability/Nodes/DOM/DOMDocument.php on line 15

I’m using php 5.4 (because that’s what centos 7 provides by default), and I suspect that’s the root cause, as the readability.php specifies 5.6 as its minimum. I’m still working to confirm.

Assuming this is right, it might be best to adjust the documented php requirements up to 5.6.


#8

yep, looks like this newer library requires php 5.6. i suppose its time to bump the minimum version required.

e: i’ve updated the wiki / dev site, although i dunno if making this a startup requirement is needed yet.


#9

Maybe an updated description inside the af_readability plugin stating the minimum PHP requirements so people know when they go to activate it?


#10

yeah this makes sense, either that or adding plugin version requirements (this seems a bit much though)


#11

Definitely overkill… but! All the classes are abstracted anyway so you could just have a is_compatible() method. In the parent class it just returns true and then individual plugins can override it and run their own checks. TT-RSS proper would just call af_readability->is_compatible() when the plugin is enabled and if it returns true it enables it, false it does not.


#12

yeah this is actually a good idea, alternatively af_readability can check php version in init an post a warning / don’t install any hooks if php < 5.6.

e: https://git.tt-rss.org/fox/tt-rss/commit/6d95e535248d91278816e88250cdeb278f6e38c2


#13

Bummer. I’m on Centos 7. Guess I’ll have to install PHP from Software Collections. Or maybe reinstall the whole VPS and install Debian instead.

Software Collections has 5.6, 7.0, and 7.1. Has anyone tried 7.1 or should I play it safe and use 7.0?


#14

Follow these instructions on DigitalOcean: https://www.digitalocean.com/community/tutorials/how-to-upgrade-to-php-7-on-centos-7


#15

I haven’t heard of IUS before. It seems to be similar to Software Collections (SCL): https://ius.io/IUSvsSCL/

Since I’m already familiar with SCL, I’ll use it (https://www.softwarecollections.org/en/scls/rhscl/rh-php70/). Thanks for the link anyway.


#16

So you want even older software?…

I am running on 7.2 and all is fine.


#17

It’s true that Debian usually has old versions, but at least it currently has PHP 7.0.

I usually prefer RHEL-based distrros, but Red Hat is really late with RHEL 8. No idea why it’s taking them so long.