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

Qt 6.5: Start hint: "Unknown error while getting elements" #7662

Open
palb91 opened this issue Apr 14, 2023 · 26 comments
Open

Qt 6.5: Start hint: "Unknown error while getting elements" #7662

palb91 opened this issue Apr 14, 2023 · 26 comments
Labels
bug: behavior Something doesn't work as intended, but doesn't crash. component: QtWebEngine Issues related to the QtWebEngine backend, based on Chromium. qt: 6 Issues related to Qt 6.

Comments

@palb91
Copy link

palb91 commented Apr 14, 2023

Version info:

qutebrowser v2.5.4
Git commit: ee40046fa-dirty on master (2023-04-11 16:29:50 +0200)
Backend: QtWebEngine 6.5, based on Chromium 108.0.5359.220 (from api)
Qt: 6.5.0

Does the bug happen if you start with --temp-basedir?:
Yes

Description
Since 2-3 days, I often have some pages where hints doesn't work and return the following message Unknown error while getting elements. Most of the time it's on a search result from duckduckgo, by I'm pretty sure I experienced the same issue with other sites (Outlook on o365 did that I think).

How to reproduce
I use qutebrowser-qt6-git from AUR on ArchLinux, running in sway (not XWayland).
Since today, I'm able to reproduce every time with $ qutebrowser -Td, then :open hello world. Once on the ddg results, pressing f returns the error message.

Stacktrace Before pressing Enter and after the error:
10:05:41 DEBUG    completion completer:_partition:171 partitioned: ['open'] 'hello world' []
10:05:41 DEBUG    completion completer:_update_completion:253 Updating completion: ['open'] hello world []
10:05:41 DEBUG    completion completer:_get_new_completion:102 Before removing flags: ['open']
10:05:41 DEBUG    completion completer:_get_new_completion:115 After removing flags: ['open']
10:05:41 DEBUG    completion completionmodel:set_pattern:182 Setting completion pattern 'hello world'
10:05:41 DEBUG    sql        histcategory:set_pattern:81 Skipping query on hello world due to prefix he returning nothing.
10:05:41 DEBUG    completion debug:__exit__:344 Set pattern hello world took 0.000417 seconds.
10:05:41 DEBUG    modes      modeman:_handle_keyrelease:340 filter: False
10:05:41 DEBUG    modes      modeman:_handle_keyrelease:340 filter: False
10:05:41 DEBUG    modes      modeman:_handle_keypress:290 got keypress in mode KeyMode.command - delegating to  passthrough=True supports_count=False win_id=0>
10:05:41 DEBUG    modes      modeman:_handle_keypress:315 match: SequenceMatch.ExactMatch, forward_unbound_keys: auto, passthrough: True, is_non_alnum: True, dry_run: True --> filter: True (focused: )
10:05:41 DEBUG    modes      modeman:_handle_keypress:290 got keypress in mode KeyMode.command - delegating to  passthrough=True supports_count=False win_id=0>
10:05:41 DEBUG    commands   command:run:535 command called: command-accept
10:05:41 DEBUG    commands   command:run:549 Calling qutebrowser.mainwindow.statusbar.command.Command.command_accept(, False)
10:05:41 DEBUG    save       savemanager:mark_dirty:73 Marking command-history as dirty.
10:05:41 DEBUG    modes      modeman:leave:434 Leaving mode KeyMode.command (reason: cmd accept)
10:05:41 DEBUG    statusbar  bar:set_mode_active:348 Setting command flag to False
10:05:41 DEBUG    completion completer:schedule_completion_update:231 Scheduling completion update.
10:05:41 DEBUG    completion completer:schedule_completion_update:228 Ignoring update because there were no changes.
10:05:41 DEBUG    statusbar  bar:_hide_cmd_widget:382 Hiding cmd widget
10:05:41 DEBUG    misc       app:on_focus_object_changed:583 Focus object changed: 
10:05:41 DEBUG    misc       app:on_focus_object_changed:583 Focus object changed: 
10:05:41 DEBUG    modes      tabbedbrowser:on_mode_left:870 Left status-input mode, focusing 
10:05:41 DEBUG    misc       app:on_focus_object_changed:583 Focus object changed: 
10:05:41 DEBUG    misc       app:on_focus_object_changed:583 Focus object changed: 
10:05:41 DEBUG    commands   command:run:535 command called: open ['hello world']
10:05:41 DEBUG    commands   command:run:549 Calling qutebrowser.browser.commands.CommandDispatcher.openurl(, 'hello world', False, False, False, False, None, False, False)
10:05:41 DEBUG    url        urlutils:get_path_if_valid:368 Checking if 'hello world' is a path
10:05:41 DEBUG    url        urlutils:is_url:272 Checking if 'hello world' is a URL (autosearch=naive).
10:05:41 DEBUG    url        urlutils:fuzzy_url:220 URL is a fuzzy search term
10:05:41 DEBUG    url        urlutils:_get_search_url:119 Finding search engine for 'hello world'
10:05:41 DEBUG    url        urlutils:_parse_search_term:106 engine None, term 'hello world'
10:05:41 DEBUG    url        urlutils:fuzzy_url:228 Converting fuzzy term 'hello world' to URL -> https://duckduckgo.com/?q=hello world
10:05:41 DEBUG    webview    browsertab:_on_before_load_started:1130 Going to start loading: https://duckduckgo.com/?q=hello world
10:05:41 DEBUG    webview    tabbedbrowser:_on_title_changed:797 Changing title for idx 0 to 'https://duckduckgo.com/?q=hello world'
10:05:41 DEBUG    modes      modeman:_handle_keypress:315 match: SequenceMatch.ExactMatch, forward_unbound_keys: auto, passthrough: True, is_non_alnum: True, dry_run: False --> filter: True (focused: )
10:05:41 DEBUG    webview    browsertab:_on_navigation_request:1154 navigation request: url https://duckduckgo.com/?q=hello world, type Type.typed, is_main_frame True
10:05:41 DEBUG    webview    webenginetab:_on_find_finished:223 Active search match: 0/0
10:05:41 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_search_match_changed(SearchMatch(current=0, total=0)) (tab 0)
10:05:41 DEBUG    statusbar  searchmatch:set_match:45 Clearing search match text.
10:05:41 DEBUG    webview    browsertab:_set_load_status:1096 load status for : LoadStatus.loading
10:05:41 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_load_status_changed() (tab 0)
10:05:41 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_load_started() (tab 0)
10:05:41 DEBUG    modes      modeman:leave:427 Ignoring leave request for KeyMode.insert (reason load started) as we're in mode KeyMode.normal
10:05:41 DEBUG    modes      tabbedbrowser:_leave_modes_on_load:777 Ignoring leave_on_load request due to setting.
10:05:42 DEBUG    modes      modeman:_handle_keyrelease:340 filter: True
10:05:42 DEBUG    js         shared:javascript_log_message:205 [:0] Error with Permissions-Policy header: Origin trial controlled feature not enabled: 'interest-cohort'.
10:05:42 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_url_changed(PyQt6.QtCore.QUrl('https://duckduckgo.com/?q=hello world')) (tab 0)
10:05:42 DEBUG    webview    tabbedbrowser:_on_title_changed:797 Changing title for idx 0 to 'https://duckduckgo.com/?q=hello world'
10:05:42 DEBUG    webview    tabbedbrowser:_on_title_changed:797 Changing title for idx 0 to 'hello world at DuckDuckGo'
10:05:43 DEBUG    webview    browsertab:_on_navigation_request:1154 navigation request: url https://duckduckgo.com/post3.html, type Type.other, is_main_frame False
10:05:43 DEBUG    js         shared:javascript_log_message:205 [:0] Error with Permissions-Policy header: Origin trial controlled feature not enabled: 'interest-cohort'.
10:05:44 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_url_changed(PyQt6.QtCore.QUrl('https://duckduckgo.com/?q=hello+world&ia=web')) (tab 0)
10:05:44 DEBUG    sql        sql:run:375 INSERT INTO History (url, title, atime, redirect) values(:url, :title, :atime, :redirect)
10:05:44 DEBUG    sql        sql:run:379     {':url': 'https://duckduckgo.com/?q=hello+world&ia=web', ':title': 'hello world at DuckDuckGo', ':atime': 1681459544, ':redirect': False}
10:05:44 DEBUG    sql        sql:run:375 REPLACE INTO CompletionHistory (url, title, last_atime) values(:url, :title, :last_atime)
10:05:44 DEBUG    sql        sql:run:379     {':url': 'https://duckduckgo.com/?q=hello+world&ia=web', ':title': 'hello world at DuckDuckGo', ':last_atime': 1681459544}
10:05:44 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_load_finished(True) (tab 0)
10:05:44 DEBUG    webview    browsertab:_set_load_status:1096 load status for : LoadStatus.success_https
10:05:44 DEBUG    signals    signalfilter:_filter_signals:87 emitting: cur_load_status_changed() (tab 0)
10:05:48 DEBUG    modes      modeman:_handle_keypress:290 got keypress in mode KeyMode.normal - delegating to 
10:05:48 DEBUG    keyboard   basekeyparser:_debug_log:216 NormalKeyParser for mode normal: Got key:  (dry_run True)
10:05:48 DEBUG    modes      modeman:_handle_keypress:315 match: SequenceMatch.ExactMatch, forward_unbound_keys: auto, passthrough: False, is_non_alnum: False, dry_run: True --> filter: True (focused: )
10:05:48 DEBUG    modes      modeman:_handle_keypress:290 got keypress in mode KeyMode.normal - delegating to 
10:05:48 DEBUG    keyboard   basekeyparser:_debug_log:216 NormalKeyParser for mode normal: Got key:  (dry_run False)
10:05:48 DEBUG    keyboard   basekeyparser:_debug_log:216 NormalKeyParser for mode normal: Definitive match for 'f'.
10:05:48 DEBUG    keyboard   basekeyparser:_debug_log:216 NormalKeyParser for mode normal: Clearing keystring (was: f).
10:05:48 DEBUG    commands   command:run:535 command called: hint
10:05:48 DEBUG    commands   argparser:multitype_conv:158 Trying to parse None as 
10:05:48 DEBUG    commands   command:run:549 Calling qutebrowser.browser.hints.HintManager.start(, 'all', , mode=None, add_history=False, rapid=False, first=False)
10:05:48 DEBUG    modes      modeman:_handle_keypress:315 match: SequenceMatch.ExactMatch, forward_unbound_keys: auto, passthrough: False, is_non_alnum: False, dry_run: False --> filter: True (focused: )
10:05:48 DEBUG    js         shared:javascript_log_message:205 [https://duckduckgo.com/?q=hello+world&ia=web:2] Uncaught TypeError: Cannot read properties of undefined (reading 'webelem')
10:05:48 DEBUG    message    message:_log_stack:55 Stack for error message:
  File "/usr/bin/qutebrowser", line 33, in 
    sys.exit(load_entry_point('qutebrowser==2.5.4', 'gui_scripts', 'qutebrowser')())
  File "/usr/lib/python3.10/site-packages/qutebrowser/qutebrowser.py", line 245, in main
    return app.run(args)
  File "/usr/lib/python3.10/site-packages/qutebrowser/app.py", line 130, in run
    ret = qt_mainloop()
  File "/usr/lib/python3.10/site-packages/qutebrowser/app.py", line 140, in qt_mainloop
    return objects.qapp.exec()
  File "/usr/lib/python3.10/site-packages/qutebrowser/browser/webengine/webenginetab.py", line 743, in _js_cb_multiple
    error_cb(webelem.Error("Unknown error while getting "
  File "/usr/lib/python3.10/site-packages/qutebrowser/browser/hints.py", line 797, in 
    error_cb=lambda err: message.error(str(err)),
  File "/usr/lib/python3.10/site-packages/qutebrowser/utils/message.py", line 73, in error
    stack = ''.join(traceback.format_stack())
10:05:48 ERROR    message    message:error:78 Unknown error while getting elements
10:05:48 DEBUG    misc       mainwindow:_update_overlay_geometry:350 new geometry for : PyQt6.QtCore.QRect(0, 1021, 960, 22)
10:05:48 DEBUG    modes      modeman:_handle_keyrelease:340 filter: True

There are already some issues about it, but most of them are closed and the other seem to use specific QT settings (#5569, #6277), while I don't (except maybe running under Wayland). So… sorry if it is still a duplicate.

@The-Compiler
Copy link
Member

This is a Qt 6.5 regression, see #7624 - but probably good to have a separate issue for this anyways.

cc @toofar @jpalus @OmeletWithoutEgg @rien333

@The-Compiler The-Compiler changed the title Start hint: "Unknown error while getting elements" Qt 6.5: Start hint: "Unknown error while getting elements" Apr 14, 2023
@The-Compiler The-Compiler added component: QtWebEngine Issues related to the QtWebEngine backend, based on Chromium. qt: 6 Issues related to Qt 6. bug: behavior Something doesn't work as intended, but doesn't crash. labels Apr 14, 2023
@The-Compiler
Copy link
Member

The-Compiler commented Apr 14, 2023

From there:

Can confirm the "Unknown error while getting elements" issue in Qt 6.5. I'm guessing that it'll happen when the url domain changes, and the error will persist in a single tab.

Your guess seems to be on track. If you open a new tab with https://www.google.com/search?q=qutebrowser, run :hint inputs, and then do a search for a new term, hinting will no longer work within that tab. And as you speculate, this process does indeed change the underlying URL.

Notably, if you duplicate that tab with gd, hinting will work again.

(note: gC is the correct binding, gd downloads the current page)

@palb91
Copy link
Author

palb91 commented Apr 14, 2023

Thanks, I'll try it.
I just tried to reproduce with the branch qt65 in a virtualenv, it surprisingly works.

@The-Compiler
Copy link
Member

I just tried to reproduce with the branch qt65 in a virtualenv, it surprisingly works.

The branch isn't meant for public use, and currently only contains CI debugging (and master contains some Python 3.12 changes in tests). So no way this could make a difference. Are you sure you were in fact using Qt 6.5 there? See :version for that.

@toofar
Copy link
Member

toofar commented Apr 14, 2023

I think we are just missing a complete reproduction setup. I suspect it is yet another weird lifecycle issue. @rien333's reproducer was pretty good but sometimes the hinting just keeps working after that, even in the same browser instance (and then it breaks again).
(Also when I have two windows open with stuff happening in the active tabs in each of them sometimes when I do stuff in one window the other one freezes until I switch tabs back and forth in the first window. Weird. I'm also running with the vulkan backend enabled etc so not the cleanest reproducer, just a hunch. Also it's really annoying)

@palb91
Copy link
Author

palb91 commented Apr 14, 2023

Are you sure you were in fact using Qt 6.5 there? See :version for that.

And… I was using Qt5.15. I thought the name was self-explanatory.

@The-Compiler
Copy link
Member

And… I was using Qt5.15. I thought the name was self-explanatory.

Not quite - both Qt 5 and 6 are supported in the master branch (and thus also in that branch). Qt 5 support will only be dropped at some later date.

@toofar
Copy link
Member

toofar commented Apr 16, 2023

I haven't found a root cause yet, but I think I've found a minimal enough reproducer that we could put it in some c++ for upstream. It seems to be something to do with navigating away from a page with an iframe? (Maybe navigating to a page on the same domain? Need to test that. And I assume it needs to be a cross domain iframe.)

Have these two pages:

index.html:

<body>
  <h1>index.html</h1>
  <a href="other.html">other.html</a>
  <iframe src='https://example.com/'>
  </iframe>
</body>

other.html

<body>
  <h1>other.html</h1>
  <a href="index.html">index.html</a>
</body>

Now start a web server in the folder they are in (eg python3 -m http.server 8877).
Open http://localhost:8877/index.html in qutebrowser running against Qt 6.5.
Confirm hinting still works.
Navigate to other.html either via a hint or a mouse click. (Edit: you can also just reload the index page.)
Confirm hints no longer work.
If you add a greasmonkey file that runs on all pages you can also see that when you get it into that state the JS no longer runs on the parent origin in that tab. It still runs inside the iframe though.

You can try this on 6.4 too and hints should keep working like usual, so it seems a clear regression.

Other places I saw this in the wild:

  • reddit (go to a subreddit, follow a "comments" link, check hints don't work)
  • ddg (like already reported, ddg foo, add some more text to the search box, search, check hints don't work)
  • google (same as ddg, although you might have to search an extra time?)

@rien333
Copy link
Contributor

rien333 commented Apr 17, 2023

New observation: when hinting breaks, caret mode also breaks. I've seen this happen a couple of times, and it happens consistently on my google reproducer (which, btw, I still haven't seen fail?). Duplicating the tab makes caret mode + hinting work again.

Caret mode behaves pretty oddly in this buggy state. For one, the text of the "modeline" changes to "CARET MODE" (like normal), but the color of the modeline doesn't change at all. Also, pressing v (to toggle caret selection mode) gives the error "error toggling caret selection". Moreover, pressing y to yank a selection shows the message "nothing to yank".

@jpalus
Copy link
Contributor

jpalus commented Apr 18, 2023

Note that a 100% reproducer for me seems to be amazon:

  1. Start qutebrowser
  2. Open amazon.com
  3. Search for any product in search bar
  4. Try to display hints on search result screen

@toofar
Copy link
Member

toofar commented Apr 22, 2023

Upstream bug report here: https://bugreports.qt.io/browse/QTBUG-113109

We may be able to mitigate some cases of this (not all of them, but it looks like the search engine case "change query param on page with iframe") by injecting the global JS at multiple injection points, currently they are injected at DocumentCreation only and those scripts are affected worse. (I feel like injecting stuff twice is a workaround we've used elsewhere?)
Edit: just gave that a quick try at DocumentReady and it didn't seem to help at all. Doing both DocumentCreation and InjectionPoint.Deferred seems to make hinting consistently work. I'm not sure why DocumentReady isn't showing the improved behaviour with qutebrowser that it is with my test script.

@The-Compiler
Copy link
Member

Looks like there is an upstream fix now: Fix user script management when subframes are present (Ibda66f55) · Gerrit Code Review

From the issue:

Scripts installed in the profile works fine, so that could be used as a workaround.

And indeed this quick hack seems to fix things:

diff --git i/qutebrowser/browser/webengine/webenginetab.py w/qutebrowser/browser/webengine/webenginetab.py
index 6f0ea82f3..57efb515b 100644
--- i/qutebrowser/browser/webengine/webenginetab.py
+++ w/qutebrowser/browser/webengine/webenginetab.py
@@ -1058,11 +1058,11 @@ def _inject_js(self, name, js_code, *,
         script.setWorldId(world)
         script.setRunsOnSubFrames(subframes)
         script.setName(f'_qute_{name}')
-        self._widget.page().scripts().insert(script)
+        self._widget.page().profile().scripts().insert(script)
 
     def _remove_js(self, name):
         """Remove an early QWebEngineScript."""
-        scripts = self._widget.page().scripts()
+        scripts = self._widget.page().profile().scripts()
         if machinery.IS_QT6:
             for script in scripts.find(f'_qute_{name}'):
                 scripts.remove(script)
@@ -1109,7 +1109,7 @@ def _inject_all_greasemonkey_scripts(self):
         self._inject_greasemonkey_scripts(scripts)
 
     def _remove_all_greasemonkey_scripts(self):
-        page_scripts = self._widget.page().scripts()
+        page_scripts = self._widget.page().profile().scripts()
         for script in page_scripts.toList():
             if script.name().startswith("GM-"):
                 log.greasemonkey.debug('Removing script: {}'
@@ -1131,7 +1131,7 @@ def _inject_greasemonkey_scripts(self, scripts):
         # make sure we replace existing scripts, not just add new ones.
         # While, taking care not to remove any other scripts that might
         # have been added elsewhere, like the one for stylesheets.
-        page_scripts = self._widget.page().scripts()
+        page_scripts = self._widget.page().profile().scripts()
         self._remove_all_greasemonkey_scripts()
 
         seen_names = set()

But that's indeed quite the hack - it should probably come with a wrapper function only doing that on Qt 6.5.0, and it probably needs some logic to not inject scripts multiple times. It's also unclear to me if there is any other logic which would break if the scripts are shared between all pages.

On the other hand, I suppose we could do this properly and move the scripts to something per-profile instead of per-tab entirely? I don't know, I don't see a reason off-hand why we'd need them to be per-tab at the moment.

@toofar what's your take on this? I'll probably be busy this/next week, so can you take it from here? Would be happy to do a quick review somewhere between things.

@toofar
Copy link
Member

toofar commented Apr 25, 2023

I don't see a reason off-hand why we'd need them to be per-tab at the moment

I agree. They are global scripts that we want everywhere. Registering them with the profiles (when we create the profiles, not in the page lifecycle hooks!) and letting Qt take care of inserting them into all the pages sounds like a good idea. I don't remember if there is a reason why we are doing it per-page, presumably it is a hangover from the pre-qtwebenginescript jseval-like insertion method (which we would still need to use for webkit).

can you take it from here?

I won't have any time to look at this until the weekend (and I think getting a release ready is more important than this tbh). If no-one has had a look at it by then I'll try to see how much work moving them up to the profiles is. But seeing as this is going to be fixed with 6.5.1 (how often do point releases come out?) if it looks like a reasonable amount of work I would lean toward the injecting them twice hack so I can get back to release stuff, as I think that would be much less work.

If anyone is building webengine themselves currently the upstream fix is working great!

@The-Compiler
Copy link
Member

QtWebEngine 6.5.1 is planned for May 10th, so only about 2 weeks away. Once the fix is merged upstream, I also plan to ask the Archlinux maintainer to add it to their QtWebEngine package, which I suppose should cover most people.

@ionenwks
Copy link

If anyone is building webengine themselves currently the upstream fix is working great!

I added the fix to Gentoo's qtwebengine and just here to +1 that it's indeed working fine 👍

pld-gitsync pushed a commit to pld-linux/qt6 that referenced this issue Apr 28, 2023
toofar added a commit that referenced this issue Apr 30, 2023
Qt had a regression relating to injecting scripts in 6.5. This only
applied to QWebEngineScripts in a page's collection, ones in a
profile's collection where fine. Since QWebEngineScripts learnt about
greasemonkey compatible metadata (a few years ago now) Qt manages when
scripts are injected and into what sites. So from that point of view all
we have to do is make sure scripts are registered with Qt when a profile
is initialised and then forget about them. So moving up to the profile
level fits that lifecycle better.

This is an initial, low effort, attempt at moving script registrations
to be up in the profile. Here's what I've done:

* move everything around QWebEngineScript out of webenginetab up to
  webenginesettings
    * injecting greasemonkey scripts
    * injecting site specific quirks (the site specific part is actually
      managed by greasemonkey metadata)
    * injecting global JS that is used on every page like hint, caret
      and stylesheet utility code
* move JS_WORLD_MAP up  to qtutils alongside MAX_WORLD_ID
    * this now introduces backend specific code into this module
* move greasemonkey initialisation to be earlier so the singleton
  manager exists when webenginesettings are initialized
    * I haven't looked at what dependancies the grasemonkey module has,
      if this causes issue we could split the greasemonkey
      initialization up (part one: create singleton, part two: read
      all scripts + fire reloaded signal)
* the profile level stylesheet is still overriden in the tab when a) a
  search is started or ended, to show/hide the scroll bar b) when the
  user stylesheets setting changes, so you don't have to reload the page

Moving everything up off of an object, in webenginetab, up to module
level function in webenginesettings meant removing a bunch of references
to "self" and using functools.partial to retain references to profiles
for signals. Subclassing QWebEngineProfile would probably help make that
stuff a little more robust, and would help us move towards having an
arbitrary number of profiles. But the only downside I can think of right
now is that signal connections wont get cleaned up when profiles are
deleted because they aren't connected to bound methods. But we aren't
currently deleting profiles (apart from at shutdown).

I left a couple of comments in around possible improvements. The
interface for the change_filter decorator surprised me a bit.

`_inject_greasemonkey_scripts()` might be able to be made smaller by
re-using the script factory. Or moving the world validation out.

Regarding the original regression in 6.5, regarding all the global
scripts like stylesheet, hint, caret etc. That issue can also be worked
around by injecting them twice (at document created and deferred). They
all have guards in the code so should be idempotent. That doesn't help
greasemonkey scripts though which are also affected.

I haven't tried running the tests :)

ref: #7662
toofar added a commit that referenced this issue Apr 30, 2023
Qt had a regression relating to injecting scripts in 6.5. This only
applied to QWebEngineScripts in a page's collection, ones in a
profile's collection where fine. Since QWebEngineScripts learnt about
greasemonkey compatible metadata (a few years ago now) Qt manages when
scripts are injected and into what sites. So from that point of view all
we have to do is make sure scripts are registered with Qt when a profile
is initialised and then forget about them. So moving up to the profile
level fits that lifecycle better.

This is an initial, low effort, attempt at moving script registrations
to be up in the profile. Here's what I've done:

* move everything around QWebEngineScript out of webenginetab up to
  webenginesettings
    * injecting greasemonkey scripts
    * injecting site specific quirks (the site specific part is actually
      managed by greasemonkey metadata)
    * injecting global JS that is used on every page like hint, caret
      and stylesheet utility code
* move JS_WORLD_MAP up  to qtutils alongside MAX_WORLD_ID
    * this now introduces backend specific code into this module
* move greasemonkey initialisation to be earlier so the singleton
  manager exists when webenginesettings are initialized
    * I haven't looked at what dependancies the grasemonkey module has,
      if this causes issue we could split the greasemonkey
      initialization up (part one: create singleton, part two: read
      all scripts + fire reloaded signal)
* the profile level stylesheet is still overriden in the tab when a) a
  search is started or ended, to show/hide the scroll bar b) when the
  user stylesheets setting changes, so you don't have to reload the page

Moving everything up off of an object, in webenginetab, up to module
level function in webenginesettings meant removing a bunch of references
to "self" and using functools.partial to retain references to profiles
for signals. Subclassing QWebEngineProfile would probably help make that
stuff a little more robust, and would help us move towards having an
arbitrary number of profiles. But the only downside I can think of right
now is that signal connections wont get cleaned up when profiles are
deleted because they aren't connected to bound methods. But we aren't
currently deleting profiles (apart from at shutdown).

I left a couple of comments in around possible improvements. The
interface for the change_filter decorator surprised me a bit.

`_inject_greasemonkey_scripts()` might be able to be made smaller by
re-using the script factory. Or moving the world validation out.

Regarding the original regression in 6.5, regarding all the global
scripts like stylesheet, hint, caret etc. That issue can also be worked
around by injecting them twice (at document created and deferred). They
all have guards in the code so should be idempotent. That doesn't help
greasemonkey scripts though which are also affected.

I haven't tried running the tests :)

ref: #7662
@rien333
Copy link
Contributor

rien333 commented May 6, 2023

I asked the arch maintainers to look at incorporating the upstream fix: https://bugs.archlinux.org/task/78426. (also: really happy that arch is making progress in moving from flyspray to gitlab 😌)

Edit: won't happen, but I guess webengine 6.5.1 is only a few days away.

@palb91
Copy link
Author

palb91 commented May 17, 2023

QtWebEngine 6.5.1 is planned for May 10th, so only about 2 weeks away. Once the fix is merged upstream, I also plan to ask the Archlinux maintainer to add it to their QtWebEngine package, which I suppose should cover most people.

Updated plan 24.5.2023: Coming soon!

toofar added a commit that referenced this issue May 21, 2023
Qt had a regression relating to injecting scripts in 6.5. This only
applied to QWebEngineScripts in a page's collection, ones in a
profile's collection where fine. Since QWebEngineScripts learnt about
greasemonkey compatible metadata (a few years ago now) Qt manages when
scripts are injected and into what sites. So from that point of view all
we have to do is make sure scripts are registered with Qt when a profile
is initialised and then forget about them. So moving up to the profile
level fits that lifecycle better.

This is an initial, low effort, attempt at moving script registrations
to be up in the profile. Here's what I've done:

* move everything around QWebEngineScript out of webenginetab up to
  webenginesettings
    * injecting greasemonkey scripts
    * injecting site specific quirks (the site specific part is actually
      managed by greasemonkey metadata)
    * injecting global JS that is used on every page like hint, caret
      and stylesheet utility code
* move JS_WORLD_MAP up  to qtutils alongside MAX_WORLD_ID
    * this now introduces backend specific code into this module
* move greasemonkey initialisation to be earlier so the singleton
  manager exists when webenginesettings are initialized
    * I haven't looked at what dependancies the grasemonkey module has,
      if this causes issue we could split the greasemonkey
      initialization up (part one: create singleton, part two: read
      all scripts + fire reloaded signal)
* the profile level stylesheet is still overriden in the tab when a) a
  search is started or ended, to show/hide the scroll bar b) when the
  user stylesheets setting changes, so you don't have to reload the page

Moving everything up off of an object, in webenginetab, up to module
level function in webenginesettings meant removing a bunch of references
to "self" and using functools.partial to retain references to profiles
for signals. Subclassing QWebEngineProfile would probably help make that
stuff a little more robust, and would help us move towards having an
arbitrary number of profiles. But the only downside I can think of right
now is that signal connections wont get cleaned up when profiles are
deleted because they aren't connected to bound methods. But we aren't
currently deleting profiles (apart from at shutdown).

I left a couple of comments in around possible improvements. The
interface for the change_filter decorator surprised me a bit.

`_inject_greasemonkey_scripts()` might be able to be made smaller by
re-using the script factory. Or moving the world validation out.

Regarding the original regression in 6.5, regarding all the global
scripts like stylesheet, hint, caret etc. That issue can also be worked
around by injecting them twice (at document created and deferred). They
all have guards in the code so should be idempotent. That doesn't help
greasemonkey scripts though which are also affected.

I haven't tried running the tests :)

ref: #7662
@palb91

This comment was marked as off-topic.

@rien333
Copy link
Contributor

rien333 commented May 27, 2023

So far so good, almost everything work well again!

I get pretty frequent crashes on 6.5.1, btw. If someone happens to land on a consistent reproducer, let me know.

@palb91
Copy link
Author

palb91 commented May 29, 2023

I still have not met any "full" crash, just some JS messages in random places just like the one I posted in my previous comment.

@The-Compiler
Copy link
Member

@palb91 Moved out to #7729.

@rien333 Could it be related to pinch zooming? I had someone in IRC report something around that. Can you maybe get a stacktrace? If you're on Archlinux, it should show up in coredumpctl list, and you should be able to do export DEBUGINFOD_URLS="https://debuginfod.archlinux.org/" followed by coredumpctl gdb PID with the PID from the list. Then, once you get a (gdb) prompt (might take a bit), use bt.

@rien333

This comment was marked as off-topic.

@The-Compiler

This comment was marked as off-topic.

@OmeletWithoutEgg

This comment was marked as off-topic.

@rien333

This comment was marked as off-topic.

@The-Compiler
Copy link
Member

The-Compiler commented Jun 3, 2023

@OmeletWithoutEgg @rien333 Thanks! Split off iinto #7732.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: behavior Something doesn't work as intended, but doesn't crash. component: QtWebEngine Issues related to the QtWebEngine backend, based on Chromium. qt: 6 Issues related to Qt 6.
Projects
None yet
Development

No branches or pull requests

7 participants