Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

bitwarden extension header buttons are not clickable #9436

Closed
kspearrin opened this issue Jun 14, 2017 · 10 comments
Closed

bitwarden extension header buttons are not clickable #9436

kspearrin opened this issue Jun 14, 2017 · 10 comments

Comments

@kspearrin
Copy link
Contributor

kspearrin commented Jun 14, 2017

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

  • Describe the issue you encountered: Parts of header buttons/anchors (where the text is drawn and above) in bitwarden extension header are not clickable.

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

  • Brave Version (revision SHA): 0.17.1dev

  • Steps to reproduce:

    1. Configure and open the bitwarden password manager
    2. Click login button
    3. Hover over and click the "Cancel" and "Log In" buttons at the top header.

image

  • Actual result:
    Notice that the parts of the button where the text is displayed and above are not clickable. If you move outside of the text into the padding area below you can click the buttons.

If you navigate directly to the extension page in the main browser window everything works as expected. Ex: chrome-extension://nngceckbapebfimnlniiiahkandclblb/popup/index.html#/login

relevant extension source:

https://github.com/bitwarden/browser/blob/master/src/popup/app/accounts/views/accountsLogin.html#L2-L11

This happens on every page of the extension that has header buttons.

Not an issue in all other browsers (Chrome, Firefox, Opera, etc.).

  • Expected result:
    Buttons are clickable all over.

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

  • Is this an issue in the currently released version? No, bitwarden is not yet in production.

  • Can this issue be consistently reproduced? Yes

  • Extra QA steps: n/a

@kspearrin
Copy link
Contributor Author

cc: @bsclifton

@jonathansampson
Copy link
Collaborator

jonathansampson commented Aug 1, 2017

I believe this is the same issue as #6015.

@jonathansampson jonathansampson added this to the 0.18.x Hotfix milestone Aug 1, 2017
@kspearrin
Copy link
Contributor Author

Also, this only seems to happen on windows. I didn't notice this on my MacBook.

@jonathansampson
Copy link
Collaborator

@kspearrin You're correct; I've never heard anybody complain about this on macOS or Linux.

@jonathansampson
Copy link
Collaborator

jonathansampson commented Aug 17, 2017

The issue here appears to be with the .tabStripContainer also having the .allowDragging class, which adds -webkit-app-region: drag to the tab-strip element. This element's region (red) overlaps the popup element's region (orange). At the point where these two elements intersect, nothing is clickable (because the area functions instead as a drag region).

image

Careful observation reveals this to be the case:

overlap

Because this issue does not repro on Mac, I suspect it may be a Windows-only implementation detail. It appears as though -webkit-app-region does not respect z-index, in the case of overlapping elements.

@kspearrin
Copy link
Contributor Author

Interesting find. I would have never guessed that was the problem just observing the issue as an outsider.

@jonathansampson
Copy link
Collaborator

jonathansampson commented Aug 17, 2017

The best solution, for now, may be to add/remove -webkit-app-region on certain elements, depending on the present window state. So, when an popup is presented, we deactivate the tabStrip as a drag region.

@bsclifton
Copy link
Member

bsclifton commented Aug 20, 2017

Here's the section of code we'd want to edit:

shouldAllowWindowDrag: (state, windowState, frame, isFocused) => {
const shieldState = require('./shieldState')
const defaultBrowserState = require('./defaultBrowserState')
const braveryPanelIsVisible = shieldState.braveShieldsEnabled(frame) &&
windowState.get('braveryPanelDetail')
const checkDefaultBrowserDialogIsVisible = isFocused &&
defaultBrowserState.shouldDisplayDialog(state)
return !state.get('contextMenuDetail') &&
!windowState.get('bookmarkDetail') &&
!windowState.getIn(['ui', 'siteInfo', 'isVisible']) &&
!braveryPanelIsVisible &&
!windowState.getIn(['ui', 'isClearBrowsingDataPanelVisible']) &&
!windowState.get('importBrowserDataDetail') &&
!windowState.getIn(['widevinePanelDetail', 'shown']) &&
!windowState.get('autofillAddressDetail') &&
!windowState.get('autofillCreditCardDetail') &&
!windowState.getIn(['ui', 'releaseNotes', 'isVisible']) &&
!checkDefaultBrowserDialogIsVisible &&
!windowState.getIn(['ui', 'noScriptInfo', 'isVisible']) &&
frame && !frame.getIn(['security', 'loginRequiredDetail']) &&
!windowState.getIn(['ui', 'menubar', 'selectedIndex'])
}
}

if we know a way to check for if the popup is showing… we can add the check there

@jonathansampson
Copy link
Collaborator

jonathansampson commented Aug 20, 2017

@bsclifton I believe popupWindowDetail in state may suffice. Checking on that now.

Update: This does indeed appear to resolve the issue. Working on a PR now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.