Closed
Bug 1373714
Opened 7 years ago
Closed 7 years ago
Touch-dragging scrollbar doesn't work on about:config and some UI elements
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla56
People
(Reporter: kats, Assigned: kats)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
dao
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
Dragging the scrollbar thumb in about:config and the library window doesn't work with touch. Bug 1341992 fixed thumb-dragging in web content but these UI elements seem to have special touch handling. This was reported via email by Joop.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → bugmail
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
The problem here is that even though gecko does handle touch-dragging the scrollbar thumb and touch-scrollbar interaction in general, the XUL widgets have touch-specific code that overrides that behaviour. Allowing touches that land in the scrollbar to pass through normal gecko flow resolves this issue.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P2
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8881415 [details] Bug 1373714 - Support touch-dragging the scrollthumb in XUL tree widgets. https://reviewboard.mozilla.org/r/152570/#review158918 ::: toolkit/content/widgets/tree.xml:690 (Diff revision 1) > <handlers> > <handler event="touchstart"> > <![CDATA[ > - if (event.touches.length > 1) { > + function isScrollbarElement(target) { > + return target.tagName == 'xul:thumb' || > + target.tagName == 'xul:slider'; I think checking nodeName would be saner since the xul: prefix is pretty arbitrary.
Attachment #8881415 -
Flags: review?(dao+bmo) → review+
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8881415 [details] Bug 1373714 - Support touch-dragging the scrollthumb in XUL tree widgets. https://reviewboard.mozilla.org/r/152570/#review158920 ::: toolkit/content/widgets/tree.xml:690 (Diff revision 1) > <handlers> > <handler event="touchstart"> > <![CDATA[ > - if (event.touches.length > 1) { > + function isScrollbarElement(target) { > + return target.tagName == 'xul:thumb' || > + target.tagName == 'xul:slider'; Also, please use double quotes. Thanks!
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #3) > > I think checking nodeName would be saner since the xul: prefix is pretty > arbitrary. Doesn't nodeName also include the "xul:" prefix here? I could check the localName and namespaceURI to be more exact, if you prefer that.
Flags: needinfo?(dao+bmo)
Comment 6•7 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5) > (In reply to Dão Gottwald [::dao] from comment #3) > > > > I think checking nodeName would be saner since the xul: prefix is pretty > > arbitrary. > > Doesn't nodeName also include the "xul:" prefix here? Oops. Yes, it does. > I could check the > localName and namespaceURI to be more exact, if you prefer that. I think just localName would be enough since trees shouldn't have arbitrary child nodes, but feel free to check namespaceURI too if you prefer.
Flags: needinfo?(dao+bmo)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
I added the namespaceURI check just to be safe. Also swapped the single quotes for double quotes as requested.
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1d652394d0f4 Support touch-dragging the scrollthumb in XUL tree widgets. r=dao
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1d652394d0f4
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Comment 11•7 years ago
|
||
Can you check that touch-dragging scrollbar on about:config and in the library window works for you in today's nightly? Thanks!
Flags: needinfo?(bekpekker)
Comment 12•7 years ago
|
||
Unfortunately I broke the screen of my tablet while I'm in Kyrgyzstan. Not sure when/if it can be fixed here at all, so testing is an issue currently... :-(
Flags: needinfo?(bekpekker)
Assignee | ||
Comment 13•7 years ago
|
||
No worries, thanks anyway! We can get QA to verify.
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8881415 [details] Bug 1373714 - Support touch-dragging the scrollthumb in XUL tree widgets. Approval Request Comment [Feature/Bug causing the regression]: APZ touch support [User impact if declined]: touch-dragging the scrollbar thumb in some XUL widgets (about:config, the bookmarks library, etc.) doesn't work [Is this code covered by automated tests?]: not really [Has the fix been verified in Nightly?]: verified locally [Needs manual test from QE? If yes, steps to reproduce]: yes please. STR in comment 0 [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: pretty low risk [Why is the change risky/not risky?]: small change, well understood code [String changes made/needed]: none
Attachment #8881415 -
Flags: approval-mozilla-beta?
Comment 15•7 years ago
|
||
Brindusa, it would be good to verify this in nightly.
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
Comment 16•7 years ago
|
||
Reproduced the issue on Nightly 56.0a1, on a build from 06.16.2017 with a Microsoft Surface Pro 2 device running Win 10 Insider Preview. Verified as fixed on latest Nightly 56.0a1, build ID 20170706060058 on Windows 8.1 x64 touch and a Microsoft Surface Pro 2 device running Win 10 Insider Preview
Comment 17•7 years ago
|
||
Comment on attachment 8881415 [details] Bug 1373714 - Support touch-dragging the scrollthumb in XUL tree widgets. fix for touch-drag on some widgets, verified in nightly, beta55+
Attachment #8881415 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/b8f32d3d5d85
Comment 19•7 years ago
|
||
Verified as fixed on Firefox 55 beta 8 on Microsoft Surface Pro 2 device running Win 10 Insider Preview.
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•