Closed Bug 772789 Opened 8 years ago Closed 7 years ago

Send Tab to Device activity resets checkboxes when device is rotated

Categories

(Firefox for Android :: Android Sync, defect, P2)

ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: liuche, Assigned: kshriram18)

References

()

Details

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

Attachments

(1 file)

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]
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
(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.
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
(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
Closed: 7 years ago
Resolution: --- → FIXED
Product: Mozilla Services → Android Background Services
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.