Closed
Bug 1424633
Opened 6 years ago
Closed 6 years ago
Can't check in to flight on aircanada.ca
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | fixed |
People
(Reporter: overholt, Assigned: emilio)
References
(Regressed 1 open bug)
Details
(Keywords: regression, site-compat)
Attachments
(2 files)
Clean profile in Nightly on Windows 10 1. Go to https://www.aircanada.com/ca/en/aco/home.html 2. Try to click in the 'FIRST NAME' field but can't I'll try to run mozregression to figure out when this regressed.
Reporter | ||
Comment 1•6 years ago
|
||
I forgot you have to click "Canada - English" on the language chooser thing and then click on "Check-in" (white on black "tab" towards the right).
Reporter | ||
Comment 2•6 years ago
|
||
mozregression landed on 1421654 but that seems unrelated. 2017-11-30 was good and 2017-12-01 was bad and I'm pretty sure 37e0bd91 was good and ec14d683 was bad. I'll try more later.
status-firefox57:
--- → unaffected
status-firefox58:
--- → ?
status-firefox59:
--- → affected
Keywords: regression,
regressionwindow-wanted
Priority: -- → P2
Reporter | ||
Comment 3•6 years ago
|
||
Hmm, but those dates don't line up with those revisions (https://hg.mozilla.org/mozilla-central/pushloghtml?rev=37e0bd91&rev=ec14d683).
Reporter | ||
Comment 4•6 years ago
|
||
Looks like if you press <Tab> you can get focus to enter the 'FIRST NAME' field with a a cursor but it doesn't gain a cursor if you click into it.
![]() |
||
Comment 5•6 years ago
|
||
The site uses a lot of @-moz-document in their css. Regression window: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4ad2150e73c0b19f1f6264a4b9ad6927986d43ce&tochange=37e0bd919af057d44c5c1410458c0f00a3653c11 Regressed by: 37e0bd919af0 Emilio Cobos Álvarez — Bug 1035091: Disable @-moz-document on author sheets on nightly and early beta. r=xidorn
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(emilio)
Updated•6 years ago
|
Keywords: site-compat
Reporter | ||
Comment 6•6 years ago
|
||
Thanks, Alice!
Assignee | ||
Comment 7•6 years ago
|
||
You need to be logged in to repro, right? Going to the "Check in now" page I can fill my first name just fine.
Flags: needinfo?(overholt)
![]() |
||
Comment 8•6 years ago
|
||
my str: 0. clear browsing history such as cache and cookie etc. 1. Go to https://www.aircanada.com/ca/en/aco/home.html , (no need login) 2. click "Canada - English" on the language chooser thing 3. click on "Check-in" (white on black "tab" towards the right). 4. click on the placeholder text 'FIRST NAME' in the input field, (not empty area)
Reporter | ||
Updated•6 years ago
|
Flags: needinfo?(overholt)
Comment 9•6 years ago
|
||
Mentioned this in the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2015/moz-document-support-will-be-dropped/
Assignee | ||
Comment 10•6 years ago
|
||
So, this is the bug they're working around. When clicking on the absolutely positioned span, we don't dispatch focus to the parent element. It's not clear to me that's expected. Olli, do you know what's supposed to happen here?
Flags: needinfo?(emilio) → needinfo?(bugs)
Assignee | ||
Updated•6 years ago
|
Attachment #8936689 -
Attachment mime type: text/plain → text/html
Assignee | ||
Comment 11•6 years ago
|
||
(In reply to Alice0775 White from comment #8) > my str: > 0. clear browsing history such as cache and cookie etc. > 1. Go to https://www.aircanada.com/ca/en/aco/home.html , (no need login) > 2. click "Canada - English" on the language chooser thing > 3. click on "Check-in" (white on black "tab" towards the right). > 4. click on the placeholder text 'FIRST NAME' in the input field, (not > empty area) And, thanks a lot Alice, indeed :)
Comment 12•6 years ago
|
||
We tab navigate using frames, so I think in this case it leads to https://searchfox.org/mozilla-central/rev/b0098afaeaad24a6752aa418ca3e86eade5fab17/dom/events/EventStateManager.cpp#3192-3195 What is the parent of an absolute positioned frame?
Flags: needinfo?(bugs)
Assignee | ||
Comment 13•6 years ago
|
||
(In reply to Olli Pettay [:smaug] (won't be in Austin) from comment #12) > We tab navigate using frames, so I think in this case it leads to > https://searchfox.org/mozilla-central/rev/ > b0098afaeaad24a6752aa418ca3e86eade5fab17/dom/events/EventStateManager. > cpp#3192-3195 > What is the parent of an absolute positioned frame? Yeah, the parent of the abspos is the nearest positioned frame, so it's wrong. Olli, is there a spec that defines this? I can fix this easily.
Flags: needinfo?(bugs)
Comment 14•6 years ago
|
||
Well, hit testing itself isn't spec'ed. But I think in this case when focus is happening using mouse, we shouldn't follow frames, but normal DOM propagation.
Flags: needinfo?(bugs)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 17•6 years ago
|
||
I'm thinking how this should be fixed... Flattened tree might be ok, or move tabindex handling to normal DOM event handling, in other words to EventTarget::PostHandleEvent. The latter would be faster since we're already iterating event targets in the path. (/me reviews some other patches first and gets back to this later.)
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8936714 [details] Bug 1424633: Decide which frame to focus using the flattened tree. https://reviewboard.mozilla.org/r/207452/#review213394 Ok, I think we should just move this to Element::PostHandleEvent. This is after all about mouse event handling, not sequental focus handling.
Attachment #8936714 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 19•6 years ago
|
||
Comment on attachment 8936714 [details] Bug 1424633: Decide which frame to focus using the flattened tree. So I tried to move it to PostHandleEvent, and I had a really bad time, in particular how to deal with suppressBlur and all that. Also, this wants to run regardless of mMultipleActionsPrevented, but only once, which would require adding a new event flag and that kind of stuff. Would you be fine merging this for now? This is effectively walking the frame tree and skipping display: contents nodes (which have no frames and thus are not focusable).
Attachment #8936714 -
Flags: review- → review?(bugs)
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8936714 [details] Bug 1424633: Decide which frame to focus using the flattened tree. https://reviewboard.mozilla.org/r/207452/#review213538 Well, I can't review this since I don't know whether HasAnyStateBits(NS_FRAME_OUT_OF_FLOW)/GetPlaceholderFrame() captures all the cases we're interested in here. In other words, does that follow the same path as what DOM event dispatch would follow.
Attachment #8936714 -
Flags: review?(bugs)
Comment hidden (mozreview-request) |
Comment 22•6 years ago
|
||
mozreview-review |
Comment on attachment 8936714 [details] Bug 1424633: Decide which frame to focus using the flattened tree. https://reviewboard.mozilla.org/r/207452/#review213598 ::: dom/events/EventStateManager.cpp:84 (Diff revision 3) > #include "nsIDOMXULDocument.h" > #include "nsIDragService.h" > #include "nsIDragSession.h" > #include "mozilla/dom/DataTransfer.h" > #include "nsContentAreaDragDrop.h" > +#include "nsPlaceholderFrame.h" leftover from previous patch?
Attachment #8936714 -
Flags: review?(bugs) → review+
Comment hidden (mozreview-request) |
Comment 24•6 years ago
|
||
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2ad057a99aae Decide which frame to focus using the flattened tree. r=smaug
Comment 25•6 years ago
|
||
Backed out changeset 2ad057a99aae (bug 1424633) for failing 2 in dom/events/test/test_focus_abspos.html r=backout on a CLOSED TREE https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2ad057a99aaebbe26e311f74a9de24b45f241b78&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable https://hg.mozilla.org/integration/autoland/rev/78bd64f9ee0892690c7a62b1a6a14b2e2081454d
Flags: needinfo?(bugs)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → emilio
Flags: needinfo?(bugs) → needinfo?(emilio)
Comment 26•6 years ago
|
||
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b83870070900 Decide which frame to focus using the flattened tree. r=smaug
Assignee | ||
Comment 27•6 years ago
|
||
Made the testcase a bit more reliable so it didn't intermittently fail.
Flags: needinfo?(emilio)
Assignee | ||
Comment 28•6 years ago
|
||
Here's a try run passing TV: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf06842008c6c7b2b7a75e56f3eec50f0877d475&selectedJob=151835085
Comment 29•6 years ago
|
||
Backed out for multiple failures on dom/events/test/test_focus_abspos.html https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b83870070900d6d4c487b830e9b1342de474142d&filter-resultStatus=testfailed&selectedJob=151856106 https://treeherder.mozilla.org/logviewer.html#?job_id=151856106&repo=autoland&lineNumber=2664 https://hg.mozilla.org/integration/autoland/rev/e35b0ca176e15a08bff817d3474f277b207f211b
Flags: needinfo?(emilio)
Assignee | ||
Comment 30•6 years ago
|
||
Sigh, the test is so flaky... :((. I'll try to make it a bit more robust...
Comment 31•6 years ago
|
||
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/d2322e0d24ef Decide which frame to focus using the flattened tree. r=smaug
Comment 32•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d2322e0d24ef
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(emilio)
Assignee | ||
Comment 33•6 years ago
|
||
I just verified I can check-in in AirCanada again :)
Reporter | ||
Comment 34•6 years ago
|
||
Thank you!
Updated•6 years ago
|
status-firefox-esr52:
--- → unaffected
You need to log in
before you can comment on or make changes to this bug.
Description
•