hannesmannerheim / qvitter

mirror. moved to https://git.gnu.io/h2p/Qvitter, send merge requests and issues there
https://git.gnu.io/h2p/Qvitter
GNU Affero General Public License v3.0
86 stars 26 forks source link

can't switch to classic on gnusocial.de and gnusocial.ch #224

Closed hannesmannerheim closed 9 years ago

hannesmannerheim commented 9 years ago

https://gnusocial.de/notice/6653367

hannesmannerheim commented 9 years ago

@mmn can you figure out why the UI is always hijacked on gnusocial.de and gnusocial.ch? i think you rewrote that part.

could it be that the prefs enable_qvitter and disable_qvitter is set as '1' and '0' and that you treat them as true and false in https://github.com/hannesmannerheim/qvitter/blob/master/QvitterPlugin.php#L133 and https://github.com/hannesmannerheim/qvitter/blob/master/QvitterPlugin.php#L134 ?

just guessing as it works on my instances

hannesmannerheim commented 9 years ago

@MarcusMoeller i'm trying stuff in the blind here, since it works on my test instances. can you update qvitter and try now?

hannesmannerheim commented 9 years ago

@MarcusMoeller, i've made some more small changes in the hope of fixing this. would be interesting to see if it made any difference!

LiohMoeller commented 9 years ago

Now it's always disabled and I cannot re-enable it.

hannesmannerheim commented 9 years ago

interesting!

hannesmannerheim commented 9 years ago

you've not written $config['site']['qvitter']['enabledbydefault'] = 'true'; instead of $config['site']['qvitter']['enabledbydefault'] = true; right?

LiohMoeller commented 9 years ago

$config['site']['qvitter']['enabledbydefault'] = true;

hannesmannerheim commented 9 years ago

ok, so we've ruled that out...

ghost commented 9 years ago

I can just say that I haven't had a problem at all disabling or enabling qvitter on my instance with latest GNU social etc.

I'm mostly confused regarding the "disabled by user" and "enabled by user" preferences. I would prefer it if there was instead just one setting (either enabled or disabled by user) which is either 1 or 0 depending on preferred logic.

And I'm too tired to think about this now. But if it's a problem of logic, use pen and paper and colorful boxes ;)

hannesmannerheim commented 9 years ago

it's not, it works on my test instances, and everywhere except gnusocial.de and gnusocial.ch

ghost commented 9 years ago

Sounds like it's not related to Qvitter nor GNU social :]

hannesmannerheim commented 9 years ago

@mmn do you use php 5.6?

ghost commented 9 years ago

@hannesmannerheim No, but neither does @MarcusMoeller

$ curl -sI https://gnusocial.ch | grep PHP X-Powered-By: PHP/5.5.9-1ubuntu4.11

$ curl -sI https://social.umeahackerspace.se | grep PHP X-Powered-By: PHP/5.5.9-1ubuntu4.11

$ curl -sI https://gnusocial.de | grep PHP X-Powered-By: PHP/5.6.9-0+deb8u1

hannesmannerheim commented 9 years ago

fan också trodde jag var nåt på spåren

LiohMoeller commented 9 years ago

In the past it worked fine on gs.de too. Are you also using latest qvitter code @mmn ?

ghost commented 9 years ago

Yes. Latest everything. Why don't you put some debug logging statements here and there and see which values are in effect?

It's impossible to remotely debug these kinds of problems, so lots more info is needed. Or root access.

hannesmannerheim commented 9 years ago

@mmn but i think we can conclude it has to do with static::settings('enabledbydefault')

since: 1) $this->hijack works since this https://github.com/hannesmannerheim/qvitter/commit/db42f1186f371274f354b7b0c4b2c3a785b544a6 had this https://github.com/hannesmannerheim/qvitter/issues/224#issuecomment-120508501 effect 2) public function initialize() works, since common_config_append('admin', 'panels', 'qvitteradm'); is executed https://quitter.se/notice/3862875 3) gnusocial.ch has $config['site']['qvitter']['enabledbydefault'] = true; and even when logged out the UI is not hijacked 3) this means that this if statement must be false: if(static::settings('enabledbydefault') === true && is_null($scoped)) { https://github.com/hannesmannerheim/qvitter/blob/master/QvitterPlugin.php#L133

so... for some reason QvitterPlugin::initialize() can't access QvitterPlugin::settings()? ...or that QvitterPlugin::initialize() can't set $this->hijack...?

LiohMoeller commented 9 years ago

On my instance the admin panel is displayed. I could define a sidebar text there. But all is in Classic Mode.

hannesmannerheim commented 9 years ago

yep, same as for vinz. i just asked that because if you had got an error, that would mean that QvitterPlugin::initialize() wasn't executed, but it is.

LiohMoeller commented 9 years ago

i'm in initialized! PHP message: $qvitter_enabled_by_user = 0 PHP message: $qvitter_disabled_by_user = 0 PHP message: $scoped = null PHP message: static::settings('enabledbydefault') === true PHP message: 1) $this->hijack_ui = false PHP message: [1] PHP message: 2) $this->hijack_ui = true" while reading response header from upstream, client: 151.80.31.141, server: gnusocial.ch, request: "GET /conversation/38396 HTTP/1.1", upstream: "fastcgi://unix:/var/run/php5-fpm.sock:", host: "gnusocial.ch"

hannesmannerheim commented 9 years ago

thanks!!

hannesmannerheim commented 9 years ago

so it doesn't have to do with the initialize() function at all :) sorry for bugging you about this @mmn!

hannesmannerheim commented 9 years ago

@MarcusMoeller there is no number three? 3) $this->hijack_ui = ?

LiohMoeller commented 9 years ago

No, just PHP message:

LiohMoeller commented 9 years ago

sorry, there is one above : 2015/07/11 14:57:59 [error] 17612#0: *58248 FastCGI sent in stderr: "PHP message: 3) $this->hijack_ui = false

hannesmannerheim commented 9 years ago

many thanks.

now try the latest regular QvitterPlugin.php file!

LiohMoeller commented 9 years ago

All works perfectly now. Thanks a lot!

hannesmannerheim commented 9 years ago

:))) i'm grateful for your patience!

@mmn it works when i moved the hijack ui check back from initialize() to onRouterInitialized() but i suspect you moved it for a very good reason in https://github.com/hannesmannerheim/qvitter/commit/c0e13272880217d72b6db20be8c81714253d5dd9 can you explain better what "conflicted because of previously not static declaration of QvitterPlugin::settings" means? i don't understand that senctence... what was conflicted..?

ghost commented 9 years ago

@hannesmannerheim the "conflicted" statement was because I cherry-pick'ed it from the crazy_stunt stuff and it was a merge conflict. (because the two different branches had different declarations of QvitterPlugin::settings).

I don't understand why it wouldn't work as it should when $this->hijack_ui is set in initialize() though.

ghost commented 9 years ago

The reason I put it in initialize() was mainly because that function exists to do exactly such preparations and initialization (having configuration and preference parsing done in onRouterInitialized is a bit unintuitive imho :])

hannesmannerheim commented 9 years ago

i also don't understand why it doesn't work in initialize() [on gnusocial.de and gnusocial.ch] but at least it works now... there's no security issue or something having it in onRouterInitialized until we understand the real problem with initialize()?

hannesmannerheim commented 9 years ago

closing this as at least it's working now...