Fixes #2653. Encode details as JSON before filling input#2658
Fixes #2653. Encode details as JSON before filling input#2658
Conversation
|
Thanks I will review this tomorrow. |
karlcow
left a comment
karlcow
left a comment
There was a problem hiding this comment.
@miketaylr dirty things indeed.
Could we make sure to leave a comment to explain why we need the dumps there?
And maybe open a new issue on processing/formatting these server side before pushing them to github.
What do you think about the textarea thing? Would it even solve something?
webcompat/form.py
Outdated
| ua_header = form_data['user_agent'] | ||
| # Populate the form | ||
| bug_form.browser.data = get_browser(ua_header) | ||
| bug_form.details.data = json.dumps(form_data.get('details')) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
@miketaylr there is a bizarre thing in your screenshot The input shows double quotes inside double quotes is it really what happens? This kind of issues are difficult to test without the right client. :) |
It's possible this is just DevTools not displaying the escaped quotes. I ran a little test to be sure:
|
Yeah, this needs a big long comment. :) Will do. |
|
Have some local stuff with this approach:
I don't love it, but it's better than the current mess. We can make it better in #2659. Hoping to get patches/tests cleaned up for review after the kids go to bed tonight. |
|
let me try something: {"json": "on one line", "what": "github does", "with them": "no idea"} |
|
ah no specific formatting. |
|
@miketaylr what about doing this before dumping in the textarea things_for_textarea = json.dumps(details_dict, indent=2) |
This changes the way we build details, and updates tests accordingly. In build_details, we first attempt to parse the details argument as JSON. If successful, we pull out the `consoleLog` key and store it to be used to build the console logs section. If JSON parsing failed, we assume we received a (legacy) string, and pass that into get_details. Inside of get_details, if we're able to access the items() member of the details dict, we return the formatted string as a group of <li>s, otherwise if we get an AttributeError (which happens for a string, or other non-dict type), we just return the string as-is. When building the console section of details, if there are not any logs, it will just render as an empty string. But if there are logs, we return a new section with the content inside a <pre> tag. This is handled by the new get_console_section method.
miketaylr
force-pushed
the
issues/2653/1
branch
from
9212c3a to
467632d
Compare
|
OK, approach slightly changed. I'm still using a hidden input here, because if we stick it inside r? @karlcow |
|
|
haha <3 |
2 participants
This is a bit voodoo magic here. I don't actually know where the
detailsPOST param gets added as the hidden input value, before this patch.It seems like a default Flask / wtform behavior?
Either way, before this patch the hidden input is getting populated with a unicode string that kinda looks like JSON, but is not:
With this patch, we end up with encoded JSON:
webcompat/webcompat-tests#1704 is a test bug I filed from my branch.
r? @karlcow
There's follow up work to make it prettier, but I'll file a new bug for that.