1281462 - Disable the modal highlight feature on Nightly for now

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect, P1)

Description

[Mike de Boer [:mikedeboer]]

See summary.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

I don't think we need to disable this on Nightly, FWIW. I was just suggesting on IRC that we should ifdef the pref to NIGHTLY_BUILD so it doesn't ride the trains on the next uplift to Aurora.

But I'll obviously defer to yours and Jared's opinion if you feel it should be fully disabled.

Comment 2

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

I'm in favor of disabling for now. We've gotten sufficient feedback that we need to either re-approach the problem or fix a large number of issues before it will become usable for many of the power users on Nightly. 

At this point, I think users may be less likely to file bugs under the assumption that their issue is already known and we may start getting less value from keeping it enabled.

Comment 3

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

Attachment: Bug 1281462 - Disable the modal highlight feature on Nightly for now.

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

Comment 4

[Mike de Boer [:mikedeboer]]

Well, I think the problem is not so much disabling the feature (which I think includes enabling highlight all by default), but the unit tests in toolkit/modules/tests/browser/ that assume this pref to be true at the start.
I'd like to disable those and confirm everything works ok with a try push. I can do that tomorrow as well - today was a bit on the busy side. What do you think?

Comment 5

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

Yeah that's fine. I think it would be better if you take this bug over then :)

Comment 6

[Mike de Boer [:mikedeboer]]

Oh, I think your patch is actually almost there :P I was more thorough than I thought!

Comment 7

[Mike de Boer [:mikedeboer]]

Steal!

Comment 8

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=48c8a2a60813

Comment 9

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1281462 - Disable the modal highlight feature on Nightly for now.

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

Comment 10

[Mike de Boer [:mikedeboer]]

Comment on attachment 8764648 [details]
Bug 1281462 - Disable the modal highlight feature on Nightly for now.

https://reviewboard.mozilla.org/r/60380/#review57414

Comment 11

[:Gijs (he/him)]

Comment on attachment 8764911 [details]
Bug 1281462 - Disable the modal highlight feature on Nightly for now.

https://reviewboard.mozilla.org/r/60516/#review57420

r=me assuming green try.

::: browser/app/profile/firefox.js
(Diff revision 1)
> -pref("findbar.highlightAll", true);
> -pref("findbar.modalHighlight", true);

It's not clear to me why we have this pref in 2 places, but I guess we're getting rid of this one - maybe if/when we re-enable, we can enable it directly in modules/libpref ?

Comment 12

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #11)
> It's not clear to me why we have this pref in 2 places, but I guess we're
> getting rid of this one - maybe if/when we re-enable, we can enable it
> directly in modules/libpref ?

I was hesitant to impose this feature on SeaMonkey/ Thunderbird right away...

Comment 13

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/integration/fx-team/rev/afb7cb42a98a66de401af5bcf7de03733f2784cd
Bug 1281462 - Disable the modal highlight feature on Nightly for now. r=Gijs

Comment 14

[Guilherme Lima]

Landed this morning: https://hg.mozilla.org/mozilla-central/rev/afb7cb42a98a

Comment 15

[Brindusa Tot, Desktop QA]

Find toolbar was enabled on Nightly 52 and disabled on other channels(Aurora 51 and Beta 50). For More detail please see bug 1303248.