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

Shift click with multiple selection behaves awkwardly in Edge #1203

Closed
maier49 opened this issue Nov 11, 2015 · 5 comments
Closed

Shift click with multiple selection behaves awkwardly in Edge #1203

maier49 opened this issue Nov 11, 2015 · 5 comments

Comments

@maier49
Copy link
Contributor

maier49 commented Nov 11, 2015

In Edge, if shift + click is used in Edge, the browser also highlights all the text from the last focused element outside of the grid body to the point that was shift clicked. This can be easily replicated in the OnDemand test page for dgrid. Simply click text outside of a grid, or a header, then click a row in the grid and shift click another row. This behavior can be avoided by calling preventDefault on the event, but I don't know what kind of unintended side effects that might have.

@kitsonk
Copy link
Member

kitsonk commented Nov 16, 2015

The current version of dgrid wasn't designed to work with Microsoft Edge, so labelling this as an enhancement.

@kfranqueiro
Copy link
Member

I intend for us to fix any Edge bugs we discover prior to tagging any new releases. How it's labeled here doesn't really matter to me :)

maier49 added a commit to maier49/dgrid that referenced this issue Dec 28, 2015
Prevents strange row highlighting in Edge when selecting
multiple rows by shift clicking.

Fixes dojo#1203
maier49 added a commit to maier49/dgrid that referenced this issue Dec 28, 2015
Prevents highlighting inappropriate HTML in Edge when selecting
multiple rows by shift clicking.

Fixes dojo#1203
@kfranqueiro
Copy link
Member

I looked at Bradley's PR, but digging further into this issue, I'm realizing that Edge is not going down the same code path as IE because Edge defines webkitUserSelect in addition to msUserSelect which throws the feature-detection code off. So while the PR fixes it, I think there might be a more appropriate solution.

I also suspect we may be able to yank out a bunch of code from Selection.js provided we require Dojo >= 1.8.2 (currently it's set up to require >= 1.8.9 anyway), but we're going to need to re-test every browser if we do that.

Gotta love it when we don't UA sniff because of how browsers cheat the UA string...and then a browser goes and cheats something else that makes feature detection just as brittle.

@kfranqueiro
Copy link
Member

Oh right. I should also mention that dgrid's css-user-select has feature is colliding (and losing) to dojo/dom's (which I added myself), and the two implementations have somewhat diverged. This is part of the code I was referring to "yanking out" in my previous comment, and we're probably better off doing so to prevent future confusion, though it may be an inconvenience to anyone who happens to be using dgrid 0.4 with Dojo 1.8.0 or 1.8.1 (but realistically nobody should be using those versions anyway; IIRC there were some pretty major issues prior to 1.8.3).

@kfranqueiro
Copy link
Member

Currently running automated tests over an alternate fix, which I've already verified manually: https://github.com/kfranqueiro/dgrid/commit/7211f6d3f261bd98fd35ff1e62b22b5ac0cdf65a

kfranqueiro pushed a commit that referenced this issue Jan 4, 2016
Edge fools feature detection by populating both msUserSelect and
WebkitUserSelect.  This change forces Edge down the same code path as IE,
and removes redundant code that is already present in all versions of
Dojo that dgrid supports now.

(cherry picked from commit 7211f6d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants