Closed Bug 1346166 Opened 7 years ago Closed 7 years ago

Enabling PointerEvent breaks toggle full screen for jwplayer

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Version:
54 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- disabled
firefox55 --- verified

People

(Reporter: hsinyi, Assigned: stone)

References

Details

(Keywords: regression)

Attachments

(4 files, 4 obsolete files)

Set isHandlingUserInput = true when dispatching pointerdown and pointerup
1.06 KB, patch
stone
: review+
Details | Diff | Splinter Review
Part2: Test full screen api by pointer events
3.04 KB, patch
Details | Diff | Splinter Review
Part1 for aurora
1.06 KB, patch
gchang
: approval-mozilla-aurora-
Details | Diff | Splinter Review
Part2 for aurora
3.08 KB, patch
gchang
: approval-mozilla-aurora-
Details | Diff | Splinter Review 
See the original report here: https://webcompat.com/issues/4964
Assignee: nobody → sshih
Did you upload wrong patch?


I wonder if we should go through also popup blocking handling for pointer events and do something similar to http://searchfox.org/mozilla-central/rev/36c8dd04de2f82576f62baeb14b135a2dd02c2dc/dom/events/Event.cpp#831-840
oops
Attachment #8846353 - Attachment is obsolete: true
(In reply to Olli Pettay [:smaug] from comment #2)
> I wonder if we should go through also popup blocking handling for pointer
> events and do something similar to
> http://searchfox.org/mozilla-central/rev/
> 36c8dd04de2f82576f62baeb14b135a2dd02c2dc/dom/events/Event.cpp#831-840
Agree with you. Created bug 1346605 for it.
Attachment #8846372 - Flags: review?(bugs)
Attachment #8846372 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Thanks for the prompt patch!
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11ce882384cc
Set isHandlingUserInput = true when dispatching pointerdown and pointerup. r=smaug
Keywords: checkin-needed
i imply that this means you think this error is now fixed?
well... there's no new update available and it still doesn't work. am i getting something wrong because i've been redirected to this thread.
(In reply to Kokujou from comment #10)
> i imply that this means you think this error is now fixed?
> well... there's no new update available and it still doesn't work. am i
> getting something wrong because i've been redirected to this thread.

The patch has been pushed to our inbound branch, yet mozilla-central. If you were using Nightly, it hasn't been there yet.
Tested on Windows 10 x64 using the STR from https://webcompat.com/issues/4964

Last good revision: 5ef0e255799d58592fbb7f70636cbb91c8b90c6a
First bad revision: e2395b3b1e16f0631700f917bdac5633132be68d
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5ef0e255799d58592fbb7f70636cbb91c8b90c6a&tochange=e2395b3b1e16f0631700f917bdac5633132be68d
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → affected
status-firefox55: --- → affected
Version: unspecified → 54 Branch
https://hg.mozilla.org/mozilla-central/rev/11ce882384cc
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Can we get a test for this? Also, please request Aurora approval on this when you get a chance :)
Flags: needinfo?(sshih)
Flags: in-testsuite?
Add a test case to check whether fullscreen API works with pointer events.
Flags: needinfo?(sshih)
Attachment #8847982 - Flags: review?(bugs)
Attachment #8847982 - Flags: review?(bugs) → review+
Add tag checkin-needed for part2
Keywords: checkin-needed
please rebase.
Flags: needinfo?(sshih)
Keywords: checkin-needed
Rebased
Attachment #8848954 - Attachment is obsolete: true
Flags: needinfo?(sshih)
Keywords: checkin-needed
still seems to have problems - renamed 1346166 -> bug-1346166-fix-part2-v5.patch
applying bug-1346166-fix-part2-v5.patch
patching file dom/events/test/pointerevents/mochitest.ini
Hunk #1 FAILED at 117
1 out of 1 hunks FAILED -- saving rejects to file dom/events/test/pointerevents/mochitest.ini.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug-1346166-fix-part2-v5.patch

could you take a look? thanks!
Flags: needinfo?(sshih)
Flags: needinfo?(sshih)
Flags: in-testsuite? → in-testsuite+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/59733053c359
Test full screen api by pointer events. r=smaug
Keywords: checkin-needed
Attached patch Part1 for aurora — — Splinter Review 

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Part2 for aurora
        — 
          — Splinter Review
      

    


  

    
    

    
  
Comment on attachment 8849373 [details] [diff] [review]
Part1 for aurora

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1331551 - Turn on PointerEvent preference on Windows nightly.
[User impact if declined]:
Fullscreen API doesn't work for pointer events.
[Is this code covered by automated tests?]:
Yes. It's covered by part2 patch for aurora.
[Has the fix been verified in Nightly?]:
Just landed to mozilla-central. Only verified with my local build.
[Needs manual test from QE? If yes, steps to reproduce]: 
No
[List of other uplifts needed for the feature/fix]:
The part2 patch for aurora (attachment 8849374 [details] [diff] [review])
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
This change set the flag inside Gecko for pointer events, which is similar to mouse events.
[String changes made/needed]:
No

        Attachment #8849373 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8849374 [details] [diff] [review]
Part2 for aurora

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1331551 - Turn on PointerEvent preference on Windows nightly
[User impact if declined]:
Fullscreen API doesn't work for pointer events.
[Is this code covered by automated tests?]:
This is the automated test to cover the code.
[Has the fix been verified in Nightly?]:
Just landed to mozilla-central. Only verified with my local build.
[Needs manual test from QE? If yes, steps to reproduce]: 
No
[List of other uplifts needed for the feature/fix]:
The part1 patch for aurora (attachment 8849373 [details] [diff] [review])
[Is the change risky?]:
No
[Why is the change risky/not risky?]:
This is the test case to verify that fullscreen API works for pointer events.
[String changes made/needed]:
No

        Attachment #8849374 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Hi Roxana, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(roxana.leitan)

    
    

    
  
Hi :stone,
Why do we need this patch in Aurora54 if the pref is only turn on in nightly?
Flags: needinfo?(sshih)

    
    

    
  
Hi Roxana, 
Can you confirmed if the issue will happen in 54 because this feature only enabled in nightly?

    
    

    
  
Verified fixed on the latest Nightly 55.0a1 (2017-03-20) on Windows 10, Ubuntu 16.04 and Mac OS X 10.10.


The issue is reproducible with the preference "dom.w3c_pointer_events.enabled" set to TRUE on the latest Aurora 54.0a2 (2017-03-20) on Windows 10, Ubuntu 16.04 and Mac OS X 10.10.
And with the preference "dom.w3c_pointer_events.enabled" set to FALSE (by default) the issue is not reproducible.

          status-firefox55:
          fixed → verified
Flags: needinfo?(roxana.leitan)

    
    

    
  
(In reply to Gerry Chang [:gchang] from comment #28)
> Hi :stone,
> Why do we need this patch in Aurora54 if the pref is only turn on in nightly?
Not sure if we should uplift it for those users who turn on the preference manually.
Flags: needinfo?(sshih)

    
    

    
  
Comment on attachment 8849373 [details] [diff] [review]
Part1 for aurora

PointerEvent preference is only enabled in nightly. Aurora54-.

        Attachment #8849373 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora-

        Attachment #8849374 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Depends on: 1357995

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: