Second click doesnt deselect the text and place the cursor in between the URL text

[srirambv]

• Did you search for similar issues before submitting this one?
  Yes

• Describe the issue you encountered:
  Second click doesnt deselect the text and place the cursor in between the URL text

• Platform (Win7, 8, 10? macOS? Linux distro?):
  Windows 10 x64

• Brave Version (revision SHA):
  Brave 0.15.0
  rev 8e63cc0

• Steps to reproduce:

  1. Visit a site in a new tab
  2. Click on the URL bar so that the complete URL is selected
  3. Click on the URL bar again to deselect the complete test, difficult to deselect and get cursor placed in between text

• Actual result:
  URL text selection is difficult on Windows

• Expected result:
  Second click should deselect the text and place the cursor in between the URL text

• Will the steps above reproduce in a fresh profile? If not what other info can be added?
  Yes

• Is this an issue in the currently released version?
  Not checked

• Can this issue be consistently reproduced?
  Yes

• Extra QA steps:
  1.
  2.
  3.

• Screenshot if needed:
  

• Any related issues:
  cc: @bsclifton

[bbondy]

@bsclifton could you triage? Seems bad but I didn't repro on linux or mac.

[srirambv]

@bbondy alternatively you can open a new tab click on one of the top tiles and before the URL is loaded click on the URLbar which causes the URL text to be selected and then subsequent clicks doesn't deselect the URL text.

[bsclifton]

@srirambv can you retest this on the new 0.15.305 release? I'm curious how it behaves after:

• Windows hitbox fixes
• URL bar behavior changes

Thanks 😄

[srirambv]

@bsclifton No changes in 0.15.305

[srirambv]

Works correct when system DPI is set to 100% on Windows

[srirambv]

Another report from user in community https://community.brave.com/t/click-in-url-bar-is-annoying/4506
Setting milestone for 0.20.x as this is a really annoying bug when DPI is not at 100%.

[bsclifton]

+1 from #9872

Issue is easily reproduced while at 125% DPI. Problem does not happen at 100% DPI

[bsclifton]

I went back to older versions and the last version where this worked as expected was 0.13.5

You can eventually click a place where the cursor does set. When logging events, I when this happens, a WM_IME_NOTIFY event is generated. When it fails this is not generated.

I think a successful click happens when you DO NOT click on the letters themselves. To test this, you can bump up the font-weight to 900 and then change the cursor to default instead of text.

[bsclifton]

I found a setting which I could toggle (via the shortcut or the pinned icon on taskbar) which fixes the issue 100%. However, it causes the UI to look blurry:

[bsclifton]

Appending the following flags does get the click working as expected... however, the app no longer is sized correctly (it renders itself at 100% rather than the scaled DPI size).
--high-dpi-support=1 --force-device-scale-factor=1

[bsclifton]

I narrowed down the issue some more. I created a simple project which only has a textbox using muon quick (using v4.4.9 of course). This works perfectly as expected. It seems problem is isolated to browser-laptop

I deleted items from the DOM and noticed that I could get the URL bar working as expected. After narrowing down more, I found that the URL bar would work as expected by removing this single line:

browser-laptop/app/renderer/components/main/main.js

Line 597 in fee82ab

ref={(node) => { this.mainWindow = node }}

edit: this might be a red herring, since removing this does cause a JavaScript error (which stops processing). But it does at least give a clue

[bsclifton]

Digging in more, the "fix" (not actually a fix- just causes URL bar to behave properly, but app is broken) I mentioned above works for the URL bar because it prevents this line from running:

browser-laptop/app/renderer/components/frame/frame.js

Line 178 in eced5c4

domUtil.appendChild(this.webviewContainer, this.webview)

The click handler for the frame webcontents may be interfering with the webcontents that the URLbar (and the rest of the UI) is using

[bsclifton]

+1 from user @stephenkru via #10840

[bbondy]

bbondy [11:04 PM]
more on this...

RenderWidgetHostInputEventRouter::RouteMouseEvent gets called for mouse down and mouse up. For the mouse down case it finds the target and adjusts the point, this is where the value goes wrong.

        root_view,
        gfx::Point(event->PositionInWidget().x, event->PositionInWidget().y),
        &transformed_point);

This calls into RenderWidgetHostViewAura::FrameSinkIdAtPoint and at which point FrameSinkIdAtPoint gets a transformed point.
But that transformed point at 175% DPI can be off by a pixel or 2 because it converts to pixels (*1.75) then floors that result, then converts to DIP (/1.75) and floors the result.

Example the y-coord is 21 when I click, it first does *1.75 to give 36.75, floors that, it is 36. It finds the FrameSinkIdAtPoint then converts back and floors at 36/1.75 == 20.57, which is floored again. 20 is later in the renderer in SelectionController is compaerd to 21.

SelectionController in webkit has on tolerance for the pixels being off by 1 or 2, it looks for an exact match only.

A simple fix would be in the SelectionController to allow a tolerance of 1-2px for resetting the cursor. An alternate fix would be to, if the transformed_point is the same as the passed in point_in_pixels, then just use the original value before we even multiplied by 1.75.

[11:09]
This is the double convert that throws it off 2 pixels. First convert is:
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/renderer_host/render_widget_host_view_aura.cc#1632
And a few lines later:
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/renderer_host/render_widget_host_view_aura.cc#1636

[11:14]
when there is no webview for the window at all, the problem doesn't reproduce. And the reason is because of this:

    RenderWidgetHostViewBase* root_view,
    const gfx::Point& point,
    gfx::Point* transformed_point) {
  // Short circuit if owner_map has only one RenderWidgetHostView, no need for
  // hit testing.
  if (owner_map_.size() <= 1) {
    *transformed_point = point;
    return root_view;
  }
...

[11:14]
we return the root_view directly and we don't do that double transformed_point conversion.

[11:15]
also just to verify these findings, I just tried with 200% DPI, and there's no problem. Because if you multiply by 2 then divide by 2 naturally the second number is always divisible by 2, so there is no double floor throwing off the pixels

[11:16]
cc @clifton ^ I'm going to fix by if the frame sink find is the same point as the input find, then don't double floor and use the original point

[11:17]
and mainly I'm fixing that way in case there's other places in webkit that do direct point equivalence checks for mouse down vs mouse up or similar.

For the SelectionController line of code, just pasting here for searchability in case we ever have to come back to it for a different similar problem. This is the line on the renderer side that fails because of the different pixel values:
https://chromium.googlesource.com/chromium/blink/+/master/Source/core/editing/SelectionController.cpp#498

bool SelectionController::handleMouseReleaseEvent(const MouseEventWithHitTestResults& event, const LayoutPoint& dragStartPos)
{
    bool handled = false;
    m_mouseDownMayStartSelect = false;
    // Clear the selection if the mouse didn't move after the last mouse
    // press and it's not a context menu click.  We do this so when clicking
    // on the selection, the selection goes away.  However, if we are
    // editing, place the caret.
    if (m_mouseDownWasSingleClickInSelection && m_selectionState != SelectionState::ExtendedSelection
        && dragStartPos == event.event().position() <------ ********

[bbondy]

This will be fixed in muon builds 4.3.19 or higher so moving to 0.18.x hotfix 2.