Send Tab to Device activity resets checkboxes when device is rotated

RESOLVED FIXED in mozilla20

Status

()

Firefox for Android
Android Sync
P2
normal
RESOLVED FIXED
5 years ago
a month ago

People

(Reporter: liuche, Assigned: Shriram (irc: Mavericks))

Tracking

unspecified
mozilla20
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=nalexander][lang=java][good first bug], URL)

Attachments

(1 attachment)

Scrolling rows out of sight will cause them to be displayed as unchecked the next time they come into view.

How this impacts sending:
- a row that's been scrolled out of view can't be unchecked again, so a tab will be sent to that device
- if the user tries to re-check a row (cleared by the bug), each attempt will result in a send tab command being sent to the the first device in the Send Tab list (due to how the code fetches checked guids)

(The checkbox-clearing problem is a result of added complexity from allowing users to select a device by clicking a row, instead of just the checkbox.)
MVC, baby, MVC.
OS: Mac OS X → Android
Hardware: x86 → ARM
Status: NEW → ASSIGNED
So, I have no idea what happened, but I could not access Bug 772789 at some point.  I relabeled the commits that were labelled 772789 to 771024, and merged to m-i; sorry about the confusion.

This is fixed by Bug 771024.
I only see this on rotation, but I'm on an inbound from this morning.
Summary: Send Tab UI will uncheck rows that are scrolled out of sight. → Send Tab to Device activity resets checkboxes when device is rotated
This is a good first bug in the sense that a new contributor will have help setting up a Fennec build environment and an android-sync build environment, and should be able to just touch SendTabActivity.java to change how things are initialized.
Whiteboard: [mentor=nalexander][lang=java][good first bug]
(Assignee)

Comment 5

5 years ago
Since it happens only on rotation as mentioned on Comment #3,
state of orientation needs to be checked, does potrait mode cause the issue ?
Then a modification to line/event or function causing checkbox reset should help.

There's some confusion on allowing user to select a row instead of a checkbox.
Is that still part of the main issue? Looking for code relevant to it.
(In reply to Shriram (irc: Mavericks) from comment #5)
> Since it happens only on rotation as mentioned on Comment #3,
> state of orientation needs to be checked, does potrait mode cause the issue ?
> Then a modification to line/event or function causing checkbox reset should
> help.

If I had to guess, this is an identical problem to Bug 755935. It's not a code problem, unless you count XML files as code!
Status: ASSIGNED → NEW
(Assignee)

Comment 7

5 years ago
Created attachment 695704 [details] [diff] [review]
Patch that fixes checkbox reset issue with Send Tab To Device activity when device's rotated
Attachment #695704 - Flags: review?(nalexander)
(In reply to Shriram (irc: Mavericks) from comment #7)
> Created attachment 695704 [details] [diff] [review]
> Patch that fixes checkbox reset issue with Send Tab To Device activity when
> device's rotated

Thanks for the patch, Shriram.  I think this looks good -- I'm pretty sure the flags were copy-pasted from a different activity that did require them -- but I want to check more closely so it'll have to wait until next week.

Updated

5 years ago
Assignee: nobody → kshriram18
Comment on attachment 695704 [details] [diff] [review]
Patch that fixes checkbox reset issue with Send Tab To Device activity when device's rotated

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

wfm.  Tested on Samsung Galaxy S2, which does not rotate the screen at all, and a Samsung Galaxy Tab, which does rotate the screen.
Attachment #695704 - Flags: review?(nalexander) → review+
This needs to land on git, as well as on an hg tree, which I am in the process of taking care of.  Thanks for the contribution, Shriram!  I'll update the ticket when this lands.
https://hg.mozilla.org/integration/mozilla-inbound/rev/642ce348bb5f

Thanks again, Shriram!  Please stay in touch -- we have lots of Android Sync bugs that need attention.
Target Milestone: --- → mozilla20
Status: NEW → ASSIGNED
(Assignee)

Comment 12

5 years ago
(In reply to Nick Alexander :nalexander from comment #11)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/642ce348bb5f
> 
> Thanks again, Shriram!  Please stay in touch -- we have lots of Android Sync
> bugs that need attention.

Thanks for landing it Nick. will definitely stay in touch :)
https://hg.mozilla.org/mozilla-central/rev/642ce348bb5f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services

Updated

a month ago
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.