serenity
serenity copied to clipboard
Ladybird+Browser: Add Address Bar search and autocomplete
This adds a more familiar style of searching across Ladybird. No need to specify ? before searching. URLs are now checked against the open public_suffix_list.dat, and if they aren't found to be valid, we fall through and use the default search engine.
Old Draft Message
Currently, a draft because This covers a lot of the codebase that I'm unfamiliar with, and id really appreciate some overall comments about things like: * PublicSuffixData generator is currently in `Libraries/Userland/LibWeb/PublicSuffix`. I'm not sure where quite to put this. But it is a somewhat logical place. * Have I covered all valid URL's? (Definitely not, but I cant think of many more)
Todo:
- [x] Fixup settings in SerenityOS
- [x] Find a better place for
ErrorOr<String> absolute_url(StringView url) - [x] Match the available list of engines on both platforms
https://github.com/SerenityOS/serenity/assets/36214028/e4cbdf53-7e79-41bd-a676-6db948dc2bc0
One comment before looking at this, according to the public suffix list, it seems they would rather we only pull it "once per day" to avoid load on their servers. Our CI would end up pulling it many more times than that per day, let alone developers using it.
The copy on publicsuffix.org, linked below, is updated daily from Github. If you wish to make your app download an updated list periodically, please use this URL and have your app download the list no more than once per day. (The list usually changes a few times per week; more frequent downloading is pointless and hammers our servers.)
One comment before looking at this, according to the public suffix list, it seems they would rather we only pull it "once per day" to avoid load on their servers. Our CI would end up pulling it many more times than that per day, let alone developers using it.
I did catch that. I don't know what would be a good solution to this. They do have a copy on GitHub that we could here. Surely GitHub could take that load?
Also, while I'm thinking about it, I should add a fallback so if the files cant be downloaded, it will revert to always attempting to load the page.
Edit: I'll do this in a later PR if that's okay.
Changes since the last review
- use GitHub hosted mirror so we don't overload the public suffix list requests of pulling once per day
- Added bypass PublicSuffixChecks when CTRL is pressed in Ladybird
In light of https://github.com/SerenityOS/serenity/blob/58b1d9c319fbafac8318bffbf41380e1c9ee1175/Userland/Libraries/LibGUI/TextBox.cpp#L185-L186 we might want to move the suffix list data into some other library to avoid linking LibWeb into LibGUI
In light of
https://github.com/SerenityOS/serenity/blob/58b1d9c319fbafac8318bffbf41380e1c9ee1175/Userland/Libraries/LibGUI/TextBox.cpp#L185-L186
we might want to move the suffix list data into some other library to avoid linking LibWeb into LibGUI
I added that FIXME When I added address highlighting, personally, I feel like the system we have now is more than adequate (Making the FIXME redundant) and that adding more granular highlighting is unnecessary. But I agree it isn't a good idea to have to link against libweb to see if a URL has a public suffix. Do you have any idea what soft of libraries would better suit it?
that adding more granular highlighting is unnecessary
I think it is very much necessary, especially in a browser but for the sake of consistency also in the whole system. I think its important to correctly reflect the internals of the engine and expose that information to the user
much necessary, especially in a browser but for the sake of consistency also in the whole system. I think its important to correctly reflect the internals of the engine and expose that in
I wasn't going to remove the FIXME, if someone else wants to add it they can by all means :^). What's your suggestion on where to relocate the PublicSuffix stuff?
I can only think of a LibPublicSuffix, but I'd ask on discord about this
Ready for review!!
Changes:
- New Library
LibPublicSuffix - Use a trie to search for matching domains
- Dont allow single top-level domains eg
comororg
Is there UI to opt out of this?
Is there UI to opt out of this?
There is now :)
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
This doesn't currently build because of very recent changes to the URL class.
Generally looks fine. The Qt-specific things I only skimmed over though.
Fixed the build issue if you want to test it out, will look into constructing a failable singleton in the morning when I've had some sleep :^)
This has a lot of conflicts. :sweat_smile:
This has a lot of conflicts. 😅
What conflicts?? (Fixed them all :^) )
This has conflicts again because the Qt files got moved. :sweat_smile:
This has conflicts again because the Qt files got moved. 😅
Pretty simple fix! should be ready for review now!!
All fixed!!
Let's goooo!