Skip to content

Conversation

rstanuwijaya
Copy link

@rstanuwijaya rstanuwijaya commented May 20, 2025

Addressing: #6979

Screen.Recording.2025-05-21.at.12.40.44.AM-2.mp4

Current issues:

  • Script is not consistently loaded
  • Does not work on some websites (youtube, google)
  • The browser must be selected first (cannot change browser focus)

@mr-cheffy
Copy link
Contributor

Could you please make it follow zen's code style? View other files such as "glance" on how to properly inject scripts into sites. Thanks!

@rstanuwijaya
Copy link
Author

I changed the implementation to use actor model, and somehow it now works on Google and Youtube!

Also, I'm experiencing high CPU usage when enabling split tab, but it seems like the issue also exists in 0f37364? (although I haven't tried clean build). If you also experience this issue, please let me know. Thanks!

@rstanuwijaya rstanuwijaya marked this pull request as ready for review May 21, 2025 19:13
@rstanuwijaya rstanuwijaya requested a review from mr-cheffy May 21, 2025 19:13
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 21, 2025
Copy link
Contributor

@mr-cheffy mr-cheffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Looks much better now.

I'm going to ask you to please remove the log functions, and that would pretty much be it!

@rstanuwijaya rstanuwijaya changed the title draft-fix: Allow scrolling on edge of the window fix: Allow scrolling on edge of the window May 21, 2025
@rstanuwijaya
Copy link
Author

Done!

I'm considering to add a feature to change selected tab as well (in split view), but it'll be on a separate PR.

@mr-cheffy
Copy link
Contributor

mr-cheffy commented May 21, 2025

Could you explain that a bit more? 😅

Also, please run npm run pretty

@mr-cheffy mr-cheffy self-requested a review May 21, 2025 19:44
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label May 21, 2025
@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label May 23, 2025
@rstanuwijaya
Copy link
Author

I added horizontal trigger as well. Should be ready now!

@rstanuwijaya
Copy link
Author

Hi @mauro-balades , is there anything else you want me to modify?

Im hoping this can get merged soon :)

@mr-cheffy
Copy link
Contributor

No, thank you. I'm just waiting for a new release cycle

@rstanuwijaya
Copy link
Author

Hi, I was trying to merge the dev branch but now I can't run the tests :(

Can anyone help to see the issue?

❯ python3 scripts/run_tests.py edgescroll
 0:00.60 INFO These variables are available in the mozinfo environment and can be used to skip tests conditionally:
 0:00.60 INFO     a11y_checks: False
 0:00.60 INFO     android: False
 0:00.60 INFO     android_version: 
 0:00.60 INFO     apple_catalina: False
 0:00.60 INFO     apple_silicon: False
 0:00.60 INFO     appname: zen
 0:00.60 INFO     arch: x86_64
 0:00.60 INFO     artifact: False
 0:00.60 INFO     asan: False
 0:00.60 INFO     automation: False
 0:00.60 INFO     bin_suffix: 
 0:00.60 INFO     bits: 64
 0:00.60 INFO     buildapp: browser
 0:00.60 INFO     buildtype: opt
 0:00.60 INFO     buildtype_guess: opt
 0:00.60 INFO     cc_type: clang
 0:00.60 INFO     ccov: False
 0:00.60 INFO     condprof: False
 0:00.60 INFO     coverage: False
 0:00.60 INFO     crashreporter: True
 0:00.60 INFO     datareporting: True
 0:00.60 INFO     dbus_enabled: True
 0:00.60 INFO     debug: False
 0:00.60 INFO     devedition: False
 0:00.60 INFO     display: wayland
 0:00.60 INFO     e10s: True
 0:00.60 INFO     early_beta_or_earlier: False
 0:00.60 INFO     fission: True
 0:00.60 INFO     gecko_profiler: True
 0:00.60 INFO     has_sandbox: True
 0:00.60 INFO     headless: False
 0:00.60 INFO     healthreport: True
 0:00.60 INFO     http2: False
 0:00.60 INFO     http3: False
 0:00.60 INFO     inc_origin_init: False
 0:00.60 INFO     is_emulator: False
 0:00.60 INFO     is_ubuntu: False
 0:00.60 INFO     isolated_process: False
 0:00.60 INFO     linux_distro: Arch Linux
 0:00.60 INFO     mda_gpu: False
 0:00.60 INFO     mozconfig: /home/stnav/Projects/others/zen-desktop/engine/mozconfig
 0:00.60 INFO     msix: False
 0:00.60 INFO     nightly_build: False
 0:00.60 INFO     normandy: True
 0:00.60 INFO     official: False
 0:00.60 INFO     opt: True
 0:00.60 INFO     os: linux
 0:00.60 INFO     os_version: Rolling
 0:00.60 INFO     pgo: False
 0:00.60 INFO     platform_guess: linux64
 0:00.60 INFO     processor: x86_64
 0:00.60 INFO     release_or_beta: True
 0:00.60 INFO     require_signing: False
 0:00.60 INFO     sessionHistoryInParent: True
 0:00.60 INFO     socketprocess_e10s: False
 0:00.60 INFO     socketprocess_networking: False
 0:00.60 INFO     stylo: True
 0:00.60 INFO     swgl: False
 0:00.60 INFO     sync: True
 0:00.60 INFO     telemetry: False
 0:00.60 INFO     tests_enabled: True
 0:00.60 INFO     toolkit: gtk
 0:00.60 INFO     topobjdir: /home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu
 0:00.60 INFO     topsrcdir: /home/stnav/Projects/others/zen-desktop/engine
 0:00.60 INFO     tsan: False
 0:00.60 INFO     ubsan: False
 0:00.60 INFO     updater: True
 0:00.60 INFO     verify: False
 0:00.60 INFO     verify_fission: False
 0:00.60 INFO     version: Arch Linux Rolling
 0:00.60 INFO     vertical_tab: False
 0:00.60 INFO     webgl_ipc: False
 0:00.60 INFO     win10_2009: False
 0:00.60 INFO     win11_2009: False
 0:00.60 INFO     wmfme: 0
 0:00.60 INFO     xorigin: False
 0:00.60 INFO Checking for ssltunnel processes...
 0:00.61 INFO Checking for xpcshell processes...
 0:00.63 SUITE_START: mochitest-browser - running 4 tests
 0:00.63 INFO Running manifest: zen/tests/edgescroll/browser.toml
 0:00.68 pid:3788037 Full command: /home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/dist/bin/pk12util -i /home/stnav/Projects/others/zen-desktop/engine/build/pgo/certs/mochitest.client -w /tmp/tmp8u5vn2x_.mozrunner/.crtdbpw -d /tmp/tmp8u5vn2x_.mozrunner
pid:3788037 pk12util: PKCS12 IMPORT SUCCESSFUL

 0:00.69 INFO MochitestServer : launching ['/home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/dist/bin/xpcshell', '-g', '/home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/dist/bin', '-e', "const _PROFILE_PATH = '/tmp/tmp8u5vn2x_.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '127.0.0.1'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false; const _HTTPD_PATH = '/home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/dist/bin/components';", '-f', '/home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/_tests/testing/mochitest/server.js']
 0:00.69 INFO runtests.py | Server pid: 3788048
 0:00.69 INFO runtests.py | Websocket server pid: 3788049
 0:00.69 INFO runtests.py | SSL tunnel pid: 3788050
Server listening on port 4443 with cert pgoserver
 0:00.74 INFO use http3 server: 0
 0:00.74 INFO runtests.py | Running with scheme: http
 0:00.74 INFO runtests.py | Running with e10s: True
 0:00.74 INFO runtests.py | Running with fission: True
 0:00.74 INFO runtests.py | Running with cross-origin iframes: False
 0:00.74 INFO runtests.py | Running with socketprocess_e10s: False
 0:00.74 INFO runtests.py | Running tests: start.

 0:00.76 INFO Application command: /home/stnav/Projects/others/zen-desktop/engine/obj-x86_64-pc-linux-gnu/dist/bin/zen -marionette -foreground -profile /tmp/tmp8u5vn2x_.mozrunner
 0:00.76 INFO runtests.py | Application pid: 3788087
 0:00.76 Started process `GECKO(3788087)`
 0:00.81 GECKO(3788087) Error: Failed to open Wayland display, fallback to X11. WAYLAND_DISPLAY='wayland-0' DISPLAY=':0'
 0:01.21 GECKO(3788087) 1752058565976   Marionette      INFO    Marionette enabled
 0:01.21 GECKO(3788087) 1752058565977   Marionette      TRACE   Received observer notification final-ui-startup
 0:01.25 GECKO(3788087) console.error: "Warning: unrecognized command line flag" "-foreground"
 0:01.27 GECKO(3788087) 1752058566035   Marionette      INFO    Listening on port 2828
 0:01.27 GECKO(3788087) 1752058566036   Marionette      DEBUG   Marionette is listening
 0:01.28 GECKO(3788087) 1752058566050   Marionette      DEBUG   Accepted connection 0 from 127.0.0.1:33920
 0:01.29 GECKO(3788087) 1752058566062   Marionette      DEBUG   Closed connection 0
 0:01.30 GECKO(3788087) 1752058566062   Marionette      DEBUG   Accepted connection 1 from 127.0.0.1:33936
 0:01.33 GECKO(3788087) 1752058566102   Marionette      DEBUG   1 -> [0,1,"WebDriver:NewSession",{"strictFileInteractability":true}]
 0:01.34 GECKO(3788087) 1752058566104   Marionette      DEBUG   Waiting for initial application window
 0:01.44 GECKO(3788087) JavaScript warning: chrome://browser/content/tabbrowser/tabs.js, line 301: unreachable code after return statement
 0:01.64 GECKO(3788087) JavaScript warning: resource:///modules/UrlbarMuxerUnifiedComplete.sys.mjs, line 799: unreachable code after return statement
 0:01.64 GECKO(3788087) console.error: "ZenThemeModifier: Error initializing browser layout" (new TypeError("can't access property \"tabContainer\", this.window.gBrowser is undefined", "resource:///modules/UrlbarInput.sys.mjs", 284))
 0:01.70 GECKO(3788087) JavaScript error: chrome://browser/content/browser-init.js, line 160: TypeError: can't access property "initPlaceHolder", gURLBar is undefined
 0:01.72 GECKO(3788087) JavaScript error: , line 0: uncaught exception: undefined
 0:01.72 GECKO(3788087) JavaScript error: , line 0: uncaught exception: undefined
 0:01.72 GECKO(3788087) JavaScript error: , line 0: uncaught exception: undefined
 0:01.73 GECKO(3788087) JavaScript error: chrome://browser/content/browser.js, line 3233: TypeError: can't access property "uiDensityChanged", gURLBar is undefined

@mr-cheffy
Copy link
Contributor

@rstanuwijaya did you import the en-US packs correctly?

@rstanuwijaya
Copy link
Author

Ah I see.. I forgot to pull l10n last time.

Now it's on sync with dev again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to import this file anywhere? Last time it seems to be imported from features.inc

modifiers,
0,
0,
0
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is aOptions = 0 ok?

It seems like this corresponds to these flags:

  const unsigned long WHEEL_EVENT_CAUSED_BY_NO_LINE_OR_PAGE_DELTA_DEVICE = 0x0001;
  const unsigned long WHEEL_EVENT_CAUSED_BY_MOMENTUM                     = 0x0002;
  const unsigned long WHEEL_EVENT_CUSTOMIZED_BY_USER_PREFS               = 0x0004;
  // If any of the following flags is specified this method will throw an
  // exception in case the relevant overflowDelta has an unexpected value.
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_X_ZERO      = 0x0010;
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_X_POSITIVE  = 0x0020;
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_X_NEGATIVE  = 0x0040;
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_Y_ZERO      = 0x0100;
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_Y_POSITIVE  = 0x0200;
  const unsigned long WHEEL_EVENT_EXPECTED_OVERFLOW_DELTA_Y_NEGATIVE  = 0x0400;

@Minemetero
Copy link

I wish this PR could be merged, maybe ping cheffy for a review

@mr-cheffy
Copy link
Contributor

It just feels like quite the complicated fix for just 4 pixels of extra scroll... I'll see what other alternatives there could be and let you know

@rstanuwijaya
Copy link
Author

No worries, i also think it's quite a lot of changes for a minor feature.

Just ping me if you want me to resolve merge conflicts later on. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants