Bugzilla

(In reply to Bob from comment #41)
> (In reply to Petru-Mugurel Lingurar [:petru] from comment #38)
> > (In reply to Bob from comment #26)
> > > Unrelated but I also noticed while looking at this:
> > > 
> > > In `UserInteractionOnBackPressedCallback`, `handleOnBackPressed` iterates through all fragments(?) and calls `onBackPressed()` on all of them. Should this instead break out of the loop on first `true` return? Currently it sets `onBackPressedHandled` but continues to fire the event on the rest of the collection. `UserInteractionHandler` doc-comment says "Returns true if this [UserInteractionHandler] consumed the event and no other components need to be notified."
> > 
> > This is a nice optimization! 
> > Would you want to add a separate patch for this in the current PR or for another ticket?
> 
> It's a trivial change to make and I believe it better matches the doc-comment and past (theoretical?) behaviour, but I actually have no idea how to go about testing this one - in my tests, there was only ever one fragment so it never loops anyway. Perhaps this is best split into a separate bug if it's even worth changing.

Sounds good!
If we'd get a new ticket and a separate patch for this we;d be able to focus testing on the back button behavior across the application and land look into landing at the beginning of a new Nightly cycle to allow more time for testing and catching any regressions.
While for the changes here the scenario is different and so will be the testing needed.

(In reply to Bob from comment #41)
> (In reply to Petru-Mugurel Lingurar [:petru] from comment #38)
> > (In reply to Bob from comment #26)
> > > Unrelated but I also noticed while looking at this:
> > > 
> > > In `UserInteractionOnBackPressedCallback`, `handleOnBackPressed` iterates through all fragments(?) and calls `onBackPressed()` on all of them. Should this instead break out of the loop on first `true` return? Currently it sets `onBackPressedHandled` but continues to fire the event on the rest of the collection. `UserInteractionHandler` doc-comment says "Returns true if this [UserInteractionHandler] consumed the event and no other components need to be notified."
> > 
> > This is a nice optimization! 
> > Would you want to add a separate patch for this in the current PR or for another ticket?
> 
> It's a trivial change to make and I believe it better matches the doc-comment and past (theoretical?) behaviour, but I actually have no idea how to go about testing this one - in my tests, there was only ever one fragment so it never loops anyway. Perhaps this is best split into a separate bug if it's even worth changing.

Sounds good!
If we'd get a new ticket and a separate patch for this we'd be able to focus testing on the back button behavior across the application and look into landing the change at the beginning of a new Nightly cycle to allow more time for testing and catching any regressions.
While for the changes here the scenario is different and so will be the testing needed.