Alex Gibson [:agibson] Reporter
	Description • 7 years ago

"Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:58.0) Gecko/20100101 Firefox/58.0"

STR:

1.) Visit https://www.mozilla.org/en-US/newsletter/
2.) Click on the <select> input to choose your country.
3.) Scroll to either the top or bottom of the range of <options> (arrow keys work well to reproduce)

Expected results:

The <select> input should stay put when you reach the end of an <options> list.

Actual results:

The <select> input moves around the screen in an amusing way.

Note: this same bug reproduces for me on any website with a <select> input.

I've attached a .gif of what happens.

	Timothy Nikkel (:tnikkel)
	Comment 1 • 7 years ago

I couldn't reproduce.

	Jeff Bryner [:jeff] (use NEEDINFO 😉)
	Comment 2 • 7 years ago

FYI: Happened to me on https://bugzilla.mozilla.org/form.recruiting in the 'cost center' select field when scrolling down the list.

	:glob ✱
	Comment 3 • 7 years ago

i can reproduce this issue easily by clicking on a select then holding the down arrow key until the selected option is the bottom-most visible item.  it only appears to be a problem on selects that require scrolling to view all options.

mozregression says:

Last good revision: 849017ffe2976f7721c20c3625d8fafeb3cce8dd
First bad revision: bccbe51cd7ca58204fd0ea96fe45b9e4b684efc3
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=849017ffe2976f7721c20c3625d8fafeb3cce8dd&tochange=bccbe51cd7ca58204fd0ea96fe45b9e4b684efc3

which points at: Bug 1414147 - Move -moz-font-smoothing-background-color style struct storage from nsStyleUserInterface to nsStyleFont / nsFont

	Markus Stange [:mstange] Assignee
	Comment 4 • 7 years ago

The regression range seems to be correct. I couldn't believe it so I double-checked.
This is the funniest and most bizarre regression I've ever caused.

	Markus Stange [:mstange] Assignee
	Comment 5 • 7 years ago

Oh, and I think I know what caused it: Dynamic changes to the -moz-font-smoothing-background-color property now trigger a reflow and not just a repaint. It looks like reflowing the popup causes it to be repositioned. This seems really brittle.

	Markus Stange [:mstange] Assignee
	Comment 7 • 7 years ago

This patch returns to the previous behavior of not triggering reflows. I did not fix the underlying XUL <select> positioning bug.

	Code Review Bot [:reviewbot]
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8926231 [details]
Bug 1414926 - Make -moz-font-smoothing-background-color changes only cause repaints, not reflows.

https://reviewboard.mozilla.org/r/197486/#review202702


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

	Emilio Cobos Álvarez (:emilio)
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8926231 [details]
Bug 1414926 - Make -moz-font-smoothing-background-color changes only cause repaints, not reflows.

https://reviewboard.mozilla.org/r/197486/#review202908

Huh, bizarre indeed... Any chance to write a test for this? Also, if there are good STR, maybe worth filing a bug for the XUL bug?

::: gfx/src/nsFont.h:146
(Diff revision 1)
>  
>    bool Equals(const nsFont& aOther) const;
>  
>    nsFont& operator=(const nsFont& aOther);
>  
> +  enum class MaxDifference : uint8_t {

nit: Maybe `StyleDifference`? Not a big deal. Other thing this could do is just returning `nsChangeHint`, or something like that. Anyway, this looks good.

	Markus Stange [:mstange] Assignee
	Comment 10 • 7 years ago

Thanks for the review!

I prefer the name MaxDifference because it makes it clear that the enum variants are ordered, and that changes that affect layout are always visual changes as well, so the variants don't need to be combined.
I didn't want to leak nsChangeHint into nsFont because the different nsChangeHint values have very precise meanings which nsFont probably shouldn't be expected to know about.

	Markus Stange [:mstange] Assignee
	Comment 11 • 7 years ago

I've filed bug 1415627 about the underlying popup positioning bug.

	Pulsebot
	Comment 12 • 7 years ago

Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/7279b117245b
Make -moz-font-smoothing-background-color changes only cause repaints, not reflows. r=emilio

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 13 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/7279b117245b

	Valentina Claudia Ona
	Comment 14 • 7 years ago

I verified this issue using Latest Nightly 58.0a1 with Build ID 20171113100232 on Mac OS X 10.12.
I will mark this as verified fixed.

	Gerry Chang [:gchang]
	Comment 15 • 7 years ago

Track 58- as it's verified.