1485063 - Move AccessibleCaret preferences to StaticPrefList.h

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P4)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

All the AccessibleCaret preferences providing default values at [1] can be moved to the layout section in StaticPrefList.h to avoid manually defining Add*VarCache in [2] and [3].

[1] https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/modules/libpref/init/all.js#5551-5598
[2] https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/layout/base/AccessibleCaret.cpp#84-90
[3] https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/layout/base/AccessibleCaretManager.cpp#103-121

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485063 Part 1 - Move preferences used in AccessibleCaret to StaticPrefsList.h

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485063 Part 2 - Move preferences used in AccessibleCaretEventHub to StaticPrefList.h

"layers.async-pan-zoom.enabled" has been enabled for all platforms for a
long time. I reword the comment to avoid confusion.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485063 Part 3 - Move preferences which enable AccessibleCaret to StaticPrefList.h

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1485063 Part 4 - Move preferences used in AccessibleCaretManager to StaticPrefList.h

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 9007906 [details]
Bug 1485063 Part 1 - Move preferences used in AccessibleCaret to StaticPrefsList.h

Mats Palmgren (:mats) has approved the revision.

Comment 6

[Mats Palmgren (inactive)]

Comment on attachment 9007908 [details]
Bug 1485063 Part 2 - Move preferences used in AccessibleCaretEventHub to StaticPrefList.h

Mats Palmgren (:mats) has approved the revision.

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 9007909 [details]
Bug 1485063 Part 3 - Move preferences which enable AccessibleCaret to StaticPrefList.h

Mats Palmgren (:mats) has approved the revision.

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 9007911 [details]
Bug 1485063 Part 4 - Move preferences used in AccessibleCaretManager to StaticPrefList.h

Mats Palmgren (:mats) has approved the revision.

Comment 9

[Mats Palmgren (inactive)]

Hmm, I wonder why we started using snake_case for StaticPrefs names in the first place...

Comment 10

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Mats Palmgren (:mats) from comment #9)
> Hmm, I wonder why we started using snake_case for StaticPrefs names in the
> first place...

Snake_case is recommanded in the document [1], and so far layout prefs has been following that. Media has different opinions to use CamelCase though. See the discussion in bug 1448222 comment 5 and bug 1448222 comment 11.

[1] https://searchfox.org/mozilla-central/rev/d4ef4e9747133aa2914aca2a15cf9df1e42a6aa0/modules/libpref/init/StaticPrefList.h#61-66

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Try looks good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=257347eccf5abf9c29e77370cdf3b8910f79605d

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b4e402e76ba6
Part 1 - Move preferences used in AccessibleCaret to StaticPrefsList.h r=mats
https://hg.mozilla.org/integration/autoland/rev/f42f853c8aa7
Part 2 - Move preferences used in AccessibleCaretEventHub to StaticPrefList.h. r=mats
https://hg.mozilla.org/integration/autoland/rev/1edf90819cb2
Part 3 - Move preferences which enable AccessibleCaret to StaticPrefList.h. r=mats
https://hg.mozilla.org/integration/autoland/rev/fdd2f4fc6d5a
Part 4 - Move preferences used in AccessibleCaretManager to StaticPrefList.h. r=mats

Comment 13

[Mats Palmgren (inactive)]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #10)
> Snake_case is recommanded in the document [1], and so far layout prefs has
> been following that. Media has different opinions to use CamelCase though.
> See the discussion in bug 1448222 comment 5 and bug 1448222 comment 11.

Thanks, I filed bug 1490161.

Comment 14

[Andrei Ciure[:aciure]]

Backed out 4 changesets (bug 1485063) for failing test-oop-extensions/test_ext_webrequest_frameId.html

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fdd2f4fc6d5a0fc0686fa4b20e09c4b0e10db134&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&fromchange=3dbc855f18d5ea8cee4af7fccca14f891b8b3816&selectedJob=198544392&filter-searchStr=Linux+x64+QuantumRender+debug+Mochitests+with+e10s+test-linux64-qr%2Fdebug-mochitest-e10s-15+M-e10s%2815%29

backout: https://hg.mozilla.org/integration/autoland/rev/91109fedb3b07acf96bc93229b18fdd6a37f35e3

Comment 15

[Andreea Pavel [:apavel]]

Additional retriggers proved this was intermittent: https://tinyurl.com/y7eo3nkg

Relanding the patches. 

Ting-Yu we apologize for this.

Comment 16

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Andreea, no worries. Thank you for keeping the tree green.

BTW, I don't see a push log for relanding yet. I'll reland my patches to autoland.

Comment 17

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Well, lando seems to have bugs to reland patch series. Filed bug 1490407. I'll reland them to inbound later.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d43869851540
Part 1 - Move preferences used in AccessibleCaret to StaticPrefsList.h r=mats
https://hg.mozilla.org/integration/mozilla-inbound/rev/99456cff7313
Part 2 - Move preferences used in AccessibleCaretEventHub to StaticPrefList.h. r=mats
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f53e771094d
Part 3 - Move preferences which enable AccessibleCaret to StaticPrefList.h. r=mats
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad3af468d26
Part 4 - Move preferences used in AccessibleCaretManager to StaticPrefList.h. r=mats

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d43869851540
https://hg.mozilla.org/mozilla-central/rev/99456cff7313
https://hg.mozilla.org/mozilla-central/rev/8f53e771094d
https://hg.mozilla.org/mozilla-central/rev/8ad3af468d26

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Backed out for causing bug 1490818.
https://hg.mozilla.org/mozilla-central/rev/2603d8004c78b79c8c8c6f789d752a11700cf841

Comment 21

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #20)
> Backed out for causing bug 1490818.
> https://hg.mozilla.org/mozilla-central/rev/
> 2603d8004c78b79c8c8c6f789d752a11700cf841

I was flipping the pref "layout.accessiblecaret.enabled_on_touch" to false in Part 3. That's why the selection won't work in bug 1490818.

Comment 22

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I'll correct Part 3, and land those patches again.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a90cc08d56d
Part 1 - Move preferences used in AccessibleCaret to StaticPrefsList.h r=mats
https://hg.mozilla.org/integration/autoland/rev/f1426745e933
Part 2 - Move preferences used in AccessibleCaretEventHub to StaticPrefList.h r=mats
https://hg.mozilla.org/integration/autoland/rev/253db3bbd64f
Part 3 - Move preferences which enable AccessibleCaret to StaticPrefList.h r=mats
https://hg.mozilla.org/integration/autoland/rev/df8cf3e01392
Part 4 - Move preferences used in AccessibleCaretManager to StaticPrefList.h r=mats

Comment 24

[Mats Palmgren (inactive)]

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #21)
> I was flipping the pref "layout.accessiblecaret.enabled_on_touch" to false
> in Part 3. 

Please try to backtrack your steps to figure out how that happened
so that you can avoid it in the future.  It's a bit scary if
unintentional code changes happens after the review like this.

Comment 25

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I make the mistake from the beginning when moving the prefs, not after the patches were reviewed. Anyway, it's my bad for not testing on a real Android device. Also filed Bug 1493317 for further investigation.

Comment 26

[Mats Palmgren (inactive)]

Ah, my bad for not catching that in the review then.

Comment 27

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9a90cc08d56d
https://hg.mozilla.org/mozilla-central/rev/f1426745e933
https://hg.mozilla.org/mozilla-central/rev/253db3bbd64f
https://hg.mozilla.org/mozilla-central/rev/df8cf3e01392