Closed Bug 1347689 Opened 7 years ago Closed 7 years ago

Intermittent GECKO(1691) | TEST-UNEXPECTED-FAIL | /tests/dom/events/test/pointerevents/pointerevent_touch-action-button-test_touch-manual.html | touch-action attribute test in element: assert_true: scroll received while shouldn't expected true got false

Categories

(Core :: DOM: Events, defect, P5)

Product:
Core
Component:
DOM: Events
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: stone)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell unknown])

Attachments

(1 file, 2 obsolete files)

bug-1347689-fix-v3.patch
17.50 KB, patch
kats
: review+
Details | Diff | Splinter Review
Priority: -- → P5
Assignee: nobody → sshih
The scroll event triggered by resetting target0.scrollLeft is dispatched after the timeout to reset the variable 'isFirstPart' in the test case, which causes this problem.

1. The test setTimeout(500ms) to reset target0.scrollLeft
2. The test setTimeout(1000ms) to reset the variable 'isFirstPart'
3. Timeout of step1 and reset target0.scrollLeft
4. Timeout of step2 and reset variable 'isFirstPart'
5. Got scroll event (which is triggered by step3)

We normally got #5 before #4 but sometimes don't.
See Also: → 1347322
the failure rate has reduced a lot!
Whiteboard: [stockwell unknown]
Attached patch bug-1347689-fix-v1.patch (obsolete) — — Splinter Review 
Followed by comment 12,
1. The test setTimeout(500ms) to reset target0.scrollLeft
2. The test setTimeout(1000ms) to reset the variable 'isFirstPart'
3. Timeout of step1, reset target0.scrollLeft and trigger scroll gesture
4. Expect to get scroll event before the second timeout triggered by step 2

I suspect that this is caused by
1. The first timeout is delayed.
2. because of #1, the scroll gesture is also delayed.
3. The second timeout comes on time (or less delayed).

The test case uses double intervals to make sure there is enough time to wait for the scroll event before continuing the testing. However, it doesn't work while the first timeout is delayed and the second one isn't.

Tweaked the test case and had a try [1], it looks good. But it's hard to make sure the patch works because of the low failure rate. Maybe we could land this and keep an eye on it.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=34b6e25258890c45f55be06acd6a4a76b1112e96

I also suspect that bug 1394228 and bug 1371671 are caused by this.
Checked the logs of Bug 1371671, it happened after bug 1394228 and got a problem to restore focus.

Checked the logs of Bug 1394228, it a timeout problem of test_touch_action.html. The last log before failure is "pointerevent_touch-action-button-test_touch-manual.html | Synthesized horizontal drag"
Attachment #8920485 - Flags: feedback?(bugmail)
Comment on attachment 8920485 [details] [diff] [review]
bug-1347689-fix-v1.patch

Review of attachment 8920485 [details] [diff] [review]:
-----------------------------------------------------------------

This looks ok to me, but we should probably make this change in the upstream webplatform test repo as well. https://github.com/w3c/web-platform-tests/tree/master/pointerevents
Attachment #8920485 - Flags: feedback?(bugmail) → feedback+
James Graham would probably know the proper procedure for upstreaming these changes. Note that we have two copies of the pointerevent_support.js in the tree, the other is in testing/web-platform/tests/pointerevents/ and that one is regularly synced from the github repo.
Attached patch bug-1347689-fix-v2.patch (obsolete) — — Splinter Review 

        Attachment #8920485 -
        Attachment is obsolete: true

    
  

        Attachment #8920929 -
        Flags: review?(bugmail)

    
    

    
  


  

        Attachment #8920929 -
        Attachment is obsolete: true

        Attachment #8920929 -
        Flags: review?(bugmail)

    
  

        Attachment #8920946 -
        Flags: review?(bugmail)

    
    

    
  
Comment on attachment 8920946 [details] [diff] [review]
bug-1347689-fix-v3.patch

Review of attachment 8920946 [details] [diff] [review]:
-----------------------------------------------------------------

Please be sure to make the changes in the upstream github repo as well.

        Attachment #8920946 -
        Flags: review?(bugmail) → review+

    
    

    
  
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #26)
> Comment on attachment 8920946 [details] [diff] [review]
> bug-1347689-fix-v3.patch
> 
> Review of attachment 8920946 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Please be sure to make the changes in the upstream github repo as well.

Confirmed with James Graham that the changes under the wpt folders will be upstream periodically.

    
    

    
  
Pushed by sshih@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/970cab99ef60
Intermittent pointerevent_touch-action-button-test_touch-manual.html. r=kats.

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/970cab99ef60
Status: NEW → RESOLVED
Closed: 7 years ago

          status-firefox58:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: