Sub6Resources / flutter_html

A Flutter widget for rendering static html as Flutter widgets (Will render over 80 different html tags!)
https://pub.dev/packages/flutter_html
MIT License
1.76k stars 815 forks source link

[WEB] unusable on 1.26.0-17.0.pre or higher using --web-renderer=html #552

Closed jlubeck closed 3 years ago

jlubeck commented 3 years ago

When trying to use flutter_html on flutter 1.26.0-17.0.pre and compiling for web, the page just stays blank and CPU usage goes up until the tab eventually crashes.

Going down to flutter 1.26.0-12.0.pre works fine. So something must have changed between those 2 versions that broken something...

Not sure what else to give for information, as nothing shows up in the logs. Probably an endless loop somewhere?

Let me know if I can help with something else...

tneotia commented 3 years ago

Technically we don't support Flutter Web, so I don't know if we can help out here. It could be some changes with nullsafety in the new Flutter versions? Just throwing darts in the dark here...

jlubeck commented 3 years ago

Yes, I noticed there is no WEB tag on https://pub.dev/packages/flutter_html but it would be a shame to disregard this since it's been working great until now

tneotia commented 3 years ago

I agree. I literally just ported my own package to Flutter Web (much, much harder than I expected), so I would not be opposed to officially porting this one as well now that I have my hands dirty.

One thing that will need to be changed is iframe rendering, as webviews are not supported in Web. This also means conditional importing will be required to prevent webview deps compiling in web, and dart:html compiling in mobile. I see there are a few issues open as well regarding crashes and null exceptions on web. Let me know if there's anything else you know of that would potentially need fixing/changing so I can get a list together and work on this later.

jlubeck commented 3 years ago

Until I run into this issue, I've been using this package on web without issues for almost a year. So I would say I haven't seen anything else that would need huge attention... Granted, I only use some basic html tags (p, ul, ol, li)

Ohh, wait, I remember. In order to use this I have to force the html renderer. I mentioned this here: https://github.com/Sub6Resources/flutter_html/issues/91#issuecomment-763052103

ngaurav commented 3 years ago

@tneotia Web support is a much needed feature. We hardly use iframes with flutter_html.

tneotia commented 3 years ago

@tneotia Web support is a much needed feature. We hardly use iframes with flutter_html.

Sure, I definitely think this feature would be awesome and lots of people have asked for it. However I don't want to rush it, and having different feature sets for different platforms can be confusing. That's the beauty of Flutter - you get the same functionality across every platform - and we want to uphold that in this package.

I've got a few PRs to update and a bug to fix, which I'm attacking today afternoon. After that if I have some time, I will begin working on something for Flutter Web with nullsafety support. If you've used this package with Flutter Web and encountered any issues, please drop them here so I can look out for them when I am adding support.

ngaurav commented 3 years ago

@tneotia have a good code sprint :) Will definitely do my part with discovering the issues.

tneotia commented 3 years ago

@jlubeck I hastily compiled a web version of the package and it works like a charm, no crashes or high CPU usage.

Flutter doctor ``` [√] Flutter (Channel beta, 1.26.0-17.6.pre, on Microsoft Windows [Version 10.0.19042.844], locale en-US) [√] Android toolchain - develop for Android devices (Android SDK version 30.0.2) [X] Chrome - develop for the web (Cannot find Chrome executable at .\Google\Chrome\Application\chrome.exe) ! Cannot find Chrome. Try setting CHROME_EXECUTABLE to a Chrome executable. [√] Android Studio (version 4.1.0) [√] VS Code (version 1.52.1) [√] Connected device (2 available) ```

image

Can you send over the HTML it crashes with? Or maybe try 17.6 pre?

jlubeck commented 3 years ago

@tneotia Did you try on 1.27.0-4.0.pre ? Also, I've been seeing your comments on the other tickets that you cannot reproduce the issues... I see you are using Windows and Edge. I'm using Mac and Chrome.

Might be harder for you to get a hold of a Mac, but can you try on Chrome first?

tneotia commented 3 years ago

I have a mac, let me transfer over there and test mac/chrome. Edge is using Chromium so I don't anticipate much difference but will try regardless.

I did not try 1.27.0-4.0, I'll also try that.

tneotia commented 3 years ago

@jlubeck working well on Channel beta, 1.25.0-8.3.pre with just <p>Test</p>, Chrome macOS

Latest beta is 1.26.0-17.7.pre, working well there as well. (Channel beta, 1.26.0-17.7.pre, on macOS 11.1 20C5061b darwin-x64, locale en-US)

image

The full-fat example app HTML works fine as well.

jlubeck commented 3 years ago

Yes, channel beta 1.25 is expected to be fine. 1.26.0-17.6.pre was crashing for me though. That was the original title in the bug, which I then changed to the lowest that was still crashing for me. 1.27 is channel dev

jlubeck commented 3 years ago

Even an empty string on data is crashing it for me. I can make a screen recording if you want... but I don't see how that will help you...

jlubeck commented 3 years ago

What flutter_html version are you using @tneotia ? in your pubspec.yml

tneotia commented 3 years ago

Yeah that probably won't help much. I'm on master for flutter_html (I'm just testing the example app, not in my own app). I temporarily upgraded to Flutter 1.27 just to test, and still no issues, everything is working beautifully (besides iframe)

jlubeck commented 3 years ago

https://www.loom.com/share/c105e227b55e47699227061067806cee

So that at least you don't think I'm crazy. Let me try flutter_html on master

jlubeck commented 3 years ago

@tneotia Ok, master still breaks it. BUT. Hopefully this is it. Check this out: Screen Shot 2021-02-24 at 5 26 45 PM

It crashes when I compile with the html renderer. If I remove that it works fine. But if i compile with canvaskit, I run into other issues, so that's a no go for me...

tneotia commented 3 years ago

Video for regular web renderer: https://streamable.com/bw96jr

Canvaskit renderer:

image

Still working great.

Html renderer:

image

So I can reproduce the white screen issue but my CPU usage in chrome is barely anything??

Also my canvaskit is working, what issues are you having?

jlubeck commented 3 years ago

@tneotia Well, at least you can reproduce the white screen issue! That's good right? At least you can work on that now.

The issues I'm having with canvaskit are not flutter_html related. I have other widgets that don't render as expected on canvaskit

But I would imagine that flutter_html should work with either renderer, right?

tneotia commented 3 years ago

I built with html renderer and served it via python, seems like stuff is happening, but something is holding it up:

image

This is baffling me. All the files seem correct too, and if I can get chrome to load the scripts it should show errors if there are any.

tneotia commented 3 years ago

@jlubeck this issue due to the flutter_svg dependency it seems https://github.com/dnfield/flutter_svg/issues/491

@erickok what do you think about making flutter_svg conditional (disabling SVG support on web) so that html rendering can work?

erickok commented 3 years ago

That seems like a good starts, yes. Until we have proper modularity. At least this makes it useable for all those people who don't need svg support.

jlubeck commented 3 years ago

@tneotia how did you even find the issue was svg since there were no errors anywhere??? Great to hear svg can be made optional!

tneotia commented 3 years ago

@jlubeck I just went through all the dependencies to see which ones supported Web officially, flutter_svg doesn't so I checked the issues page and this one was recently created so I noticed it.

If you really need html renderer support you can fork the package and remove that dependency along with the code usages and you should be good to go.

@erickok the white screen error will still show up using html renderer even if the user isn't using svgs in their html. This is just the result of the dependency being compiled in the first place. That's why I was asking if we should do conditional importing to remove the dependency so that we can support the html renderer for now.

erickok commented 3 years ago

I understand and yes, just like you did with the iframe support, it would be good to have it conditional.

jlubeck commented 3 years ago

@tneotia I was about to fork and try that, but since @erickok is giving you the green light to do the conditional import, should I just wait? Do you have an estimate on when you will be able to work on that? No rush at all! Just wondering, Thank you!!

tneotia commented 3 years ago

I probably won't be able to get to it today but if everything goes smoothly I'll have it ready tomorrow.

My only worry is that even though the imports wont be there in the web code after I switch to conditional, the package itself will still get compiled, so I'll have to see if that route is going to work. I don't exactly know whether just including the package in pubspec will compile it or if it has to be used in the code.

jlubeck commented 3 years ago

@tneotia do you know what it means that they closed that issue? https://github.com/dnfield/flutter_svg/issues/491#issuecomment-786101184

jlubeck commented 3 years ago

Oh, looks like it was a Flutter issue, not even their issue. And it got merged here: https://github.com/flutter/engine/pull/24574 So whenever that is included in Dev or Beta branches, it would in theory automatically fix this issue as well?

tneotia commented 3 years ago

I think so, and just as a safety measure probably the latest version of flutter_svg. In that case I don't think I should bother with making svg conditional as it will be out to dev/beta within the next week.

As stated earlier if you need the changes immediately I would just fork and remove flutter_svg.

jlubeck commented 3 years ago

Agreed, we should probably wait a bit

erickok commented 3 years ago

Agreed

jlubeck commented 3 years ago

Ok, I just tested it on the dev branch that was released today and everything worked fine. If someone else wants to double check before I close it, that would be great!

tneotia commented 3 years ago

@erickok this is working for me as well, I think this can be closed.