Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logic to set debug styling of the GUI is incorrect #7131

Closed
Wachizungu opened this issue Mar 2, 2021 · 5 comments
Closed

logic to set debug styling of the GUI is incorrect #7131

Wachizungu opened this issue Mar 2, 2021 · 5 comments
Labels
S: diagnosed Status: diagnosed. A technical diagnosis has been performed on this issue T: bug Type: bug report: This issue describes unexpected behaviour

Comments

@Wachizungu
Copy link
Contributor

Wachizungu commented Mar 2, 2021

Work environment

Questions Answers
Type of issue Bug
OS version (server) Ubuntu
OS version (client) N/A
PHP version 7.2
MISP version / git hash 2.4.139, 7431bee
Browser N/A

Expected behavior

setting debug to 'Debug on' leads to setting debugOn class for certain GUI elements.

Actual behavior

setting debug to 'Debug on' leads to setting debugOff class for certain GUI elements.

Steps to reproduce the behavior

Set setting debug to Debug on. Inspect element on footer, check the classes assigned to the footer div. It will be "footer debugOff".

Notes

I don't really understand why this difference in classes is there so didn't want to push a potential fix myself without asking input. Many different view pages have conditional formatting based on $debugMode (footer is one of them).

Three options for debug setting are:
Debug off -> 0
Debug on -> 1
Debug + SQL dump -> 2
image

Culprit line / function seems to be:
https://github.com/MISP/MISP/blob/2.4/app/Controller/AppController.php#L789
Should be (Configure::read('debug') >= 1) instead of (Configure::read('debug') > 1) I think.

If for some reason the conditional formatting should only be done for Debug + SQL dump then this issue can just be closed and I'll document it somewhere.

@Wachizungu Wachizungu added the needs triage This issue has been automatically labelled and needs further triage label Mar 2, 2021
@enjeck enjeck added S: diagnosed Status: diagnosed. A technical diagnosis has been performed on this issue T: bug Type: bug report: This issue describes unexpected behaviour and removed needs triage This issue has been automatically labelled and needs further triage labels Mar 3, 2021
adulau added a commit that referenced this issue Mar 27, 2021
chg: [UI] fix debugon for debug = 1. fix #7131
@iglocska
Copy link
Member

Ok that was actually mostly correct - the reason being that debug = 2 alters the layout of the page by inserting the SQL dump.

@Wachizungu
Copy link
Contributor Author

Okay, thanks for the follow up. Can change it back if better that way, maybe adding a comment.

@iglocska
Copy link
Member

It depends - did you see any issues that got fixed with the change? Maybe we need to separate out the logic of changing the layout vs some other changes.

@Wachizungu
Copy link
Contributor Author

No, when I spotted this I was actually trying to debug the shortcuts / keybinds popup, which is hidden with debug on, leading to me trying to find out why / how it gets set and me finding it strange it only got set for debug = 2.

There is some styling that depends on debugOff, I didn't immediately understand what it was doing which is why I opened this issue and asked if this sounded like a bug on gitter. Looking back at it a bit, unless I'm missing something the differences are mostly minor (except for the footer part for SQL dump).

@iglocska
Copy link
Member

I think the main difference used to be a relative vs absolute offset causing the old style flash messages to be inserted straight into the menu - the debugOn mode used to add a 50px offset to address this. No longer relevant with the modern flash messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S: diagnosed Status: diagnosed. A technical diagnosis has been performed on this issue T: bug Type: bug report: This issue describes unexpected behaviour
Projects
None yet
Development

No branches or pull requests

3 participants