Mangadex search functions not working on Edge :fixed:

[Soveregin]

On edge whenever I try to search through tags checkboxes result in no tags entered at all, so only the dropdowns seem to work, and whenever I try multiple tags a plethora of commas appear in the search bar, this for some reason disables the tag inclusion mode, for example:
Edge: https://mangadex.org/search?tag_mode_exc=,,,,,,,,,any&tag_mode_inc=,,,,,,,,all&tags_inc=,,,,,,2,3
Other browsers: https://mangadex.org/search?tag_mode_exc=any&tag_mode_inc=all&tags_inc=2,3
Also when I attempt to search by the original language, even if I haven't put anything else at all it shows up as "Notice: There are no titles found with your search criteria. You can add the title here," this also occurs with searching via publication status, however with demographic it seems to just ignore that I've entered it.

 

[Nolonar]

I can confirm this, and even reported it once before.

Quite honestly, the mangadex devs don't really seem to care about MS Edge, so don't get your hopes up.
The only time they ever fixed an Edge bug, was when I took the trouble to find the precise line that caused the issue, including the code to fix it. That was when arrow-navigation had stopped working for Edge.

The problem is with how they handle the search form data.
Instead of using the standard approach, they're trying to serialize the data by hand, which is definitely non-standard.
In fact, just looking at the code raises a couple of red flags:

[ol]
[*] The code is inline instead of in its own .js file, meaning it can't be cached (because the page is dynamic and changes all the time).
[*] Everything on this site uses jQuery, except for the search code. It's bad because it's inconsistent. Even Quick Search uses jQuery.
[*] They're trying to do the serialization by hand using "new FormData(this)" and adding to it, or removing from it, instead of trusting a well-established library (jQuery) to do it with "$(this).serialize()". Additional minus points because they're actually using jQuery for everything except this code!
[*] They're even overwriting the default behavior to... perform the default behavior. "evt.preventDefault()" followed by "window.location.href = [...]"? You normally don't "preventDefault()" on a form submit, unless you're using AJAX, or you're validating the form (and validation failed). This code is essentially stopping the redirection, to perform a redirection. It's absurd!
[*] The entire search code is wrapped inside a try-catch that does NOTHING when an error triggers. Like, why?
[/ol]

Honestly, you guys (mangadex devs) seriously need to rethink the entire search code.
And by "search code", I mean the one from https://mangadex.org/search not the Quick Search one that's accessible from everywhere.

Although, to be fair, the Quick Search code could use some work too. You seem to be using 2 different quick search forms (a normal one, and a mobile one). Instead of binding the same function to both events, you just copy-pasted the function. BAD!
And don't set "location.href"... can't you just change "form.action" instead and let the browser serialize the arguments itself?

 

[bocchi]

@Nolonar just wait until Microsoft rebuilds Edge with Chromium

 

[Nolonar]

I mean, not supporting Edge is their right.
But "not supporting Edge" is not a valid reason for writing shit code.
And if writing good code will fix all browsers, including those more obscure than Edge, why not go for it?

If nothing else, do it for performance and security.
Forms are a basic part of the web. They're THE way to transmit arbitrary data from a browser to the server.
Surely, whatever server software is running will have a standard deserialization code? Code, that's been maintained and improved over decades, optimized for performance, and penetration-tested by devs and community? So why re-invent the wheel, butcher performance, break compatibility, and leave yourself open to all sorts of vulnerabilities?

Honestly, it should be a no-brainer.
And seeing all those webdevs writing horrible, horrible code just because "it works in Chrome, and f*** Edge", is just unsettling.

 

[bocchi]

It also works in firefox

 

[Teasday]

@Nolonar The js on the site overall is a complete mess because of how Holo wrote things in the early days. I haven't had the interest to go over the entire thing fixing everything yet. Also, getting rid of jQuery completely is something I plan to do, so the stuff I've written explicitly avoids using it.

In this particular case the search code is inline just because I was just lazy and simply overwrote the old stuff and forgot to bundle it later. Whoops.
jQuery.serialize wouldn't actually give the result I want there. You'll notice that the checkboxes in particular are a custom three-way solution, so blindly relying on jQuery serializing them obviously won't do any good.
The preventDefault/redirection is also because of the three-way checkboxes (which have to get "converted" from tags_both to tags_exc/tags_inc depending on their state) and overall to clean up the resulting url so it's not huge mess that looks like /search?title=&author=&artist=&lang_id=&demo_id=&status_id=&tags_inc[]=&tags_exc[]=&tag_mode_inc=&tag_mode_exc= for an empty search, not to mention the repeating tags_inc[]= parameters if I just let it go without condensing.
Additionally, the try-catch lets the form at least work naturally in case something fails (which is why evt.preventDefault is just before the end of the block), which is what seems to happen for Edge because it apparently doesn't implement some basic FormData methods. Clearly it wasn't graceful enough to work regardless, but that's what you get when you use nonstandard checkboxes.

That all said, I rewrote this stuff for now, both the search and quick search since it wasn't too much trouble and I really should've done it months ago regardless. Sorry that I forgot. Quick search especially is going to get majorly redesigned in the hopefully near future, so I didn't bother changing the basic mechanism since that would involve some annoying backend changes.

The update isn't on the site at the time of writing, but will be sooner or later.

 

[Nolonar]

@Teasday
As long as it's being worked on, everything's fine.
We'll be here to remind you if you forget
Actually, no, I'm too lazy to bother. As long as the site works, I'm happy, regardless of the state of the code.


About those custom tristate checkboxes:
I think using 3 hidden radio buttons per checkbox should greatly simplify serialization. You wouldn't have to bother with serializing the checkbox then.


Cleaning up the search string sounds like a sensible reason to mess with the arguments, though I feel like this should be done server-side instead.
At least in ASP.NET, I could simply pass an object as argument for a redirect, and the server would automatically filter out all members that are NULL, so the browser won't have to display them.
Unfortunately, it's been years since I last worked with ASP.NET, so my memory is probably not the most reliable here. Also, I have no idea how to do something like that on other types of servers, so I can't help with that.


Of course, those are just suggestions. You're the one who decides how to implement it in the end.
Best of luck.

 

[Teasday]

@Soveregin Okay, the search works on at least the latest version of Edge for me.

@Nolonar Not that it's particularly important to do right now, but you're probably right about the radio button stuff, I'll think about rewriting that. Same with redirects from the server side, might probably be a decent idea too, so I'll keep it in mind but it wasn't necessary for now.