Day 1: Merging pull requests, and a stupid bug

Today was my first day of working on QtWebEngine, and as mentioned in the previous post I started with merging some pull requests before they diverge too much because of all the planned refactoring.

In the morning I was in the train without an internet connection, so I worked on adding a few missing tests:

  • Spawning external editor
  • Some test pages for manual inspection for hint rendering, as that's difficult to automate. For some of those I attemted to create minimal examples but failed, if you know your way around HTML/CSS your help would be appreciated! See #1550.

I then merged a pretty old pull request helping with hints positioning in some corner cases, where those tests really helped.

After doing so I cleaned up the current webelem code a bit (which will also help when porting hints to QtWebEngine) and really confused myself with zoom adjustments for hints.

When drawing hints, the coordinates need to be adjusted "negatively" for the zoom level of the web view, so that the hint gets clicked with the correct position.

When getting the element coordinates via javascript however (like the PR did if possible), they are already adjusted accordingly. Since the code drawing the hints needs the unadjusted position, the zoom level needs to be adjusted in the other direction.

To top off the confusion, in the unit tests I'm not actually getting the element position via javascript, so I had to adjust the coordinates a third time so the fake getElementRects() method would act like in real-life.

After doing so, some tests failed with a strange error - the position of the rectangles was adjusted, but the width and height was not:

assert QRect(5, 5, 4, 4) == QRect(10, 10, 4, 4)

After a lot of head-scratching, I found the culprit:

{
    "left": geometry.left() - scroll_x / zoom,
    "top": geometry.top() - scroll_y / zoom,
    "right": geometry.right() - scroll_x / zoom,
    "bottom": geometry.bottom() - scroll_y / zoom,
    "width": geometry.width() / zoom,
    "height": geometry.height() / zoom,
}

This was fixed by adding the needed parentheses:

{
    "left": (geometry.left() - scroll_x) / zoom,
    "top": (geometry.top() - scroll_y) / zoom,
    "right": (geometry.right() - scroll_x) / zoom,
    "bottom": (geometry.bottom() - scroll_y) / zoom,
    "width": geometry.width() / zoom,
    "height": geometry.height() / zoom,
}

In hindsight it's - as always - totally obvious, but it was a nightmare to debug.

Since those tests (checking if zoom was accounted for properly) used a scroll position of 0/0, this evaluated to geometry.left() - (0 / zoom) which is geometry.left(). The width and height were correct because they didn't subtract any scroll position...

But the frustration was worth it, hints are now drawn correctly in various corner cases!

After that, I merged and closed some more pull requests:

  • I improved how zooming behaves with a too big count and thus superseded/closed #1140.
  • I picked out a fix for tab indicators disappearing when using :tab-move, and then closed #697 (which was pretty much a year old...) because the other things are either already fixed in the meantime, or things I disagree with.
  • I added some missing bits and tests for #1460 which allows to use partial commands (like :bac) if they're unique in the command bar.
  • And some other bits and bytes like updating pinned test dependencies or removing a few lines unused code.

Now, only 10 pull requests are still open, of which two are work-in-progress, and four are explicitly marked as postponed until after QtWebEngine support.

I plan to merge the other 6 pull requests and release a v0.7 over the next day(s), and then all is ready to start! \o/