1400595 - Unable (again) to scroll Preferences if search engine table is not scrollable

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Itiel]

This is a regression from 2017-07-17 to 2017-07-18.

All needed info is on bug 1348828, where this issue was once fixed.
I suggest to "copy" the scrolling mechanism from the "Applications" section on General tab, this way bug 1356398 would also get fixed (the Applications table seems to behave perfectly in everyt scenario).

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

[Tracking Requested - why for this release]: this looks to be too late for Firefox 56 but we should fix it for Firefox 57.

Pushlog, https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=85928fde7206dbd063b4ef7c656eb7e5e835a770&tochange=5654a60499973490633adb7eb715467a99894622

Regressed by bug 1365133

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1400595 - Allow Preferences search engine table scrollable only if its items are overflow

Review commit: https://reviewboard.mozilla.org/r/181638/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/181638/

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8910148 [details]
Bug 1400595 - Allow Preferences search engine table scrollable only if its items are overflow

https://reviewboard.mozilla.org/r/181638/#review187106

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e0277058bf34
Allow Preferences search engine table scrollable only if its items are overflow r=jaws

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Thanks!

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift to mozilla-release? We can still get this into the 56 RC2 build.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e0277058bf34

Comment 9

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8910148 [details]
Bug 1400595 - Allow Preferences search engine table scrollable only if its items are overflow

Approval Request Comment
[Feature/Bug causing the regression]: bug 1365133
[User impact if declined]: minor impact 
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: minor
[Why is the change risky/not risky?]: it doesn't impact on usability
[String changes made/needed]: none

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8910148 [details]
Bug 1400595 - Allow Preferences search engine table scrollable only if its items are overflow

Simple fix to make this scrollable, new regression from 56, let's uplift for 56 RC2

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/eb2c8ca51537

Comment 12

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Using the steps provided in https://bugzilla.mozilla.org/show_bug.cgi?id=1348828#c0, the initial issue is still reproducible on 56.0 build 4 (20170922200134) using Windows 10 x64, macOS 10.12	and Ubuntu 16.04 x64, as the One-Click Search Engines table blocks the scrolling mechanism on the Search pane.  
Ricky, any thoughts about this?

Comment 13

[Ricky Chien [:rickychien] (inactive)]

According to comment 10, the fix has uplifted to 56 RC2, I'm not sure your version is right. You might have to try again on latest version.

After preference Reorg, the search engine table has been moved from General to Search category, please make sure you're testing in about:preferences#search. 

Note that this behavior only take effect when items of One-Click Search Engines table don't exceed its table height, otherwise scrolling effect will take place in One-Click Search Engines table.

If you still see the unexpected result, you could upload the screencast to let me know. Thanks :)

Comment 14

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

(In reply to Ricky Chien [:rickychien] from comment #13)
> According to comment 10, the fix has uplifted to 56 RC2, I'm not sure your
> version is right. You might have to try again on latest version.
> 
> After preference Reorg, the search engine table has been moved from General
> to Search category, please make sure you're testing in
> about:preferences#search. 
> 
> Note that this behavior only take effect when items of One-Click Search
> Engines table don't exceed its table height, otherwise scrolling effect will
> take place in One-Click Search Engines table.
> 
> If you still see the unexpected result, you could upload the screencast to
> let me know. Thanks :)

As I mentioned in comment 12, we tested the scrolling mechanism from the Search pane. 
Investigated again on the latest 56.0 build 5 (20170925181605), using Windows 10 x64. The One-Click Search Engines table items don't exceed the table height, but at the default window size, the about:preferences#search page isn't scrollable, so we resized the window and then followed the steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1348828#c0. As you can see in the screencast https://goo.gl/4HR1jJ, the (wheel) scrolling is blocked when the cursor reaches the One-Click Search Engines table.

Comment 15

[Ricky Chien [:rickychien] (inactive)]

Attachment: uplift-release.patch

RyanVM, can you help uplift this patch again?

Due to the removal of preferences/in-content-new folder, the uplift in comment 11 has no effect on new preferences.

No worry to do revert the changes of comment 11 since it's still valid in fixing old preferences if someone switches back to old one.

Thanks

Comment 16

[Ricky Chien [:rickychien] (inactive)]

Iulia Cristescu,

Thank you for reporting this issue. I've uploaded correct patch of uplifting release. Please verify that after merging. Thanks

Comment 17

[Ryan VanderMeulen [:RyanVM]]

I can do the revised uplift, but it's highly unlikely it's going to make 56.0 at this point. We just built the final RC yesterday.

Comment 18

[Ricky Chien [:rickychien] (inactive)]

Oh, sounds like it's too late for us to fix it in fx56. We'll have to dispense with this revised uplift and let it fix in fx57. I hope that it's reasonable since it's just a minor issue and won't have impact on usability.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

It's either that or landing it now and letting it ship in the first dot release (56.0.1). Liz, do you have opinions on this?

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

From comment 1, sounds like this is minor enough that we can ship 56 without it.

Comment 21

[Camelia Badau [:cbadau], Desktop QA]

Verified fixed on Windows 7 x64, macOs 10.13 and Ubuntu 16.04 using Firefox 57 Beta 8 (buildID: 20171013042429).