Touch event is not correct when the orientation changes

RESOLVED FIXED in mozilla15

Status

()

Core
Widget
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kanru, Assigned: kanru)

Tracking

({regression})

unspecified
mozilla15
All
Gonk (Firefox OS)
regression
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
STR:

 1. rotate the device (sgs2)
 2. the homescreen is in landscape mode
 3. but the touch events are still in portrait coordinate.
It looks like methods handling input coordinates need to consider current screen rotation and make adjustments accordingly. There is already a quick access to current rotation via nsScreenGonk, but width and height are problems. Querying screen manager seems not a good idea in such performance sensitive code.
(Assignee)

Comment 2

5 years ago
Created attachment 616871 [details] [diff] [review]
Bug 745077 - Reconfigure InputReader on screen rotation.
Assignee: nobody → kchen
Attachment #616871 - Flags: review?(mwu)

Updated

5 years ago
Component: General → Widget
Product: Boot2Gecko → Core
QA Contact: general → general

Comment 3

5 years ago
Regression from bug 737199
Depends on: 737199
Keywords: regression

Comment 4

5 years ago
Comment on attachment 616871 [details] [diff] [review]
Bug 745077 - Reconfigure InputReader on screen rotation.

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

Looks good. r=me with the requested changes.

::: widget/gonk/nsAppShell.cpp
@@ +704,5 @@
>      write(signalfds[1], "w", 1);
>  }
>  
> +void
> +nsAppShell::NotifyScreenChangesInternal()

Inline this into NotifyScreenChanges. Also rename NotifyScreenChanges to NotifyScreenRotation.
Attachment #616871 - Flags: review?(mwu) → review+
(Assignee)

Comment 5

5 years ago
(In reply to Michael Wu [:mwu] from comment #4)
> > +void
> > +nsAppShell::NotifyScreenChangesInternal()
> 
> Inline this into NotifyScreenChanges. Also rename NotifyScreenChanges to
> NotifyScreenRotation.

Hmm.. I need to access the private mReader. Do you want to expose gAppShell to nsWindow? 

NotifyScreenChanges does not only notify the orientation change but also the screen size change. But if you think NotifyScreenRotation fits here then I'm okay as well.

Comment 6

5 years ago
(In reply to Kan-Ru Chen [:kanru] from comment #5)
> (In reply to Michael Wu [:mwu] from comment #4)
> > > +void
> > > +nsAppShell::NotifyScreenChangesInternal()
> > 
> > Inline this into NotifyScreenChanges. Also rename NotifyScreenChanges to
> > NotifyScreenRotation.
> 
> Hmm.. I need to access the private mReader. Do you want to expose gAppShell
> to nsWindow? 
> 

NotifyScreenChanges is part of nsAppShell, so it should be able to access mReader, no?

> NotifyScreenChanges does not only notify the orientation change but also the
> screen size change. But if you think NotifyScreenRotation fits here then I'm
> okay as well.

Yeah, it's really the only reason why the screen size would change, so I think notifyscreenrotation is fine.
(Assignee)

Comment 7

5 years ago
(In reply to Michael Wu [:mwu] from comment #6)
> > Hmm.. I need to access the private mReader. Do you want to expose gAppShell
> > to nsWindow? 
> > 
> 
> NotifyScreenChanges is part of nsAppShell, so it should be able to access
> mReader, no?

Oh, yes. So I will do gAppShell->mReader->... in it.
 
> > NotifyScreenChanges does not only notify the orientation change but also the
> > screen size change. But if you think NotifyScreenRotation fits here then I'm
> > okay as well.
> 
> Yeah, it's really the only reason why the screen size would change, so I
> think notifyscreenrotation is fine.

Got it.
(Assignee)

Comment 8

5 years ago
Created attachment 618160 [details] [diff] [review]
Reconfigure InputReader on screen rotation. v1.1
Attachment #616871 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/26dbb2e9d91d
https://hg.mozilla.org/mozilla-central/rev/26dbb2e9d91d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.