1383205 - [e10s] long drop down menu results in constant beach ball

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Nils Ohlmeier [:drno]]

With Fx 56.0a1 (2017-07-19) (64-bit) on OS X 10.12.6 with e10s on.

- Open this URL https://www.prague-airport-transfers.co.uk/zakaznik/index.php
- Open the "Booking type" drop down and choose "Transfer to Prague Airport"
- Wait for page to finish loading
- In the line where it says "From" open the right of the two drop downs which says "Prague City Center"
- Wait for the drop down to load/appear - this is already super slow
- Once the drop down appears try to scroll in it

I get a beach ball for several seconds. Once it stops beach balling I try scrolling again. Same beach ball.


Now try the same thing in a non-e10s window: it works fast and smooth.

Comment 1

[Timothy Nikkel (:tnikkel)]

Profiling seems to show the time is being spent doing roughly the same things as in bug 1118086. So maybe the list of options is so long it gets split into chunks so we are still adding options after the dropdown is open? If you keep waiting the dropdown does eventually become responsive.

Comment 2

[Nils Ohlmeier [:drno]]

Oh, looks like I really was to impatient. Indeed if I leave the drop down untouched after opening for several seconds it becomes usable.
But most users (like I did in this case) will have switched browsers by that time already.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

So there are various inefficient things that we do in the <select> implementation (which I intend to fix in separate bugs), but this specific beach-balling testcase is a regression from bug 1352963. What happens is that this is already a pretty big dropdown list (2075 items), and the <select> element has a CSS transition for border and box-shadow (pretty common on the web for select elements when they get :focus)

This generates 5 transitionend events (border-top-color, border-bottom-color, etc., and box-shadow), so this ends up triggering 6 full menu rebuilds instead of just one.

It's easy to fix that using the deferred timer that already exists, and also whitelisting only the CSS properties that we actually support to trigger this.


(This testcase also falls into another pathological bad case related to the label colors, but I'll file a separate bug for that)

Comment 4

[:Felipe Gomes (needinfo for replies!)]

(probably not worth uplifting for 55 since a menulist with 2000 items should not be common)

Comment 5

[:Felipe Gomes (needinfo for replies!)]

Attachment: Bug 1383205 - Update transitionend properties of <option> elements on a deferred task.

Also, we only care about the properties that we support and that are animatable, as the non-animatable ones do not generate transitionend events

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

Comment 6

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

Comment on attachment 8891354 [details]
Bug 1383205 - Update transitionend properties of <option> elements on a deferred task.

https://reviewboard.mozilla.org/r/162552/#review167938

::: toolkit/modules/SelectContentHelper.jsm:368
(Diff revision 1)
>            this.global.sendAsyncMessage("Forms:HideDropDown", {});
>            this.uninit();
>          }
>          break;
>        case "transitionend":
> -        this._update();
> +        if (SUPPORTED_PROPERTIES.indexOf(event.propertyName) != -1) {

SUPPORTED_PROPERTIES.includes(event.propertyName)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a6100bac239
Update transitionend properties of <option> elements on a deferred task. r=jaws

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing browser_selectpopup_colors.js, at least on Windows:

https://hg.mozilla.org/integration/mozilla-inbound/rev/731889ddbd52a3abde5ebcc766e09e9f536434e5

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5a6100bac2393e0eb99b7e0f4feeb3b806fde7f7&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=119093834&repo=mozilla-inbound

20:54:09     INFO -  97 INFO Entering test bound test_textshadow_of_options_is_dependent_on_transitionend
20:54:09     INFO -  98 INFO Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,%3Chtml%3E%3Chead%3E%3Cstyle%3E%20%20select%20%7B%20transition%3A%20all%20.1s%3B%20%7D%20%20select%3Afocus%20%7B%20text-shadow%3A%200%200%200%20%23303030%3B%20%7D%3C/style%3E%3C/head%3E%3Cbody%3E%3Cselect%20id%3D%27one%27%3E%20%20%3Coption%3E%7B%22text-shadow%22%3A%20%22none%22%7D%3C/option%3E%20%20%3Coption%20selected%3D%22true%22%3E%7B%22end%22%3A%20%22true%22%7D%3C/option%3E%3C/select%3E%3C/body%3E%3C/html%3E" line: 0}]
20:54:09     INFO -  99 INFO Console message: [JavaScript Warning: "Error in parsing value for ‘font-size’.  Declaration dropped." {file: "chrome://browser/content/browser.xul" line: 0 column: 0 source: "NaNpx"}]
20:54:09     INFO -  100 INFO Console message: [JavaScript Warning: "Error in parsing value for ‘font-size’.  Declaration dropped." {file: "chrome://browser/content/browser.xul" line: 0 column: 0 source: "NaNpx"}]
20:54:09     INFO -  101 INFO Console message: [JavaScript Warning: "Error in parsing value for ‘font-size’.  Declaration dropped." {file: "chrome://browser/content/browser.xul" line: 0 column: 0 source: "NaNpx"}]
20:54:09     INFO -  102 INFO Console message: [JavaScript Warning: "Error in parsing value for ‘font-size’.  Declaration dropped." {file: "chrome://browser/content/browser.xul" line: 0 column: 0 source: "NaNpx"}]
20:54:09     INFO -  103 INFO <select> has text-shadow: rgb(48, 48, 48) 0px 0px 0px
20:54:09     INFO -  104 INFO TEST-PASS | browser/base/content/test/forms/browser_selectpopup_colors.js | Correct number of items -
20:54:09     INFO -  105 INFO TEST-PASS | browser/base/content/test/forms/browser_selectpopup_colors.js | The first child should not be selected -
20:54:09     INFO -  Buffered messages finished
20:54:09    ERROR -  106 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/forms/browser_selectpopup_colors.js | Item 1 has correct foreground color - Got rgb(0, 0, 0), expected undefined
20:54:09     INFO -  Stack trace:
20:54:09     INFO -  chrome://mochikit/content/browser-test.js:test_is:1002
20:54:09     INFO -  chrome://mochitests/content/browser/browser/base/content/test/forms/browser_selectpopup_colors.js:testOptionColors:203
20:54:09     INFO -  chrome://mochitests/content/browser/browser/base/content/test/forms/browser_selectpopup_colors.js:testSelectColors:277
20:54:09     INFO -  Not taking screenshot here: see the one that was previously logged

Comment 9

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

Comment on attachment 8891354 [details]
Bug 1383205 - Update transitionend properties of <option> elements on a deferred task.

https://reviewboard.mozilla.org/r/162552/#review168028

::: browser/base/content/test/forms/browser_selectpopup_colors.js:147
(Diff revision 1)
> +const SELECT_TEXTSHADOW_OF_OPTION_CHANGES_AFTER_TRANSITIONEND =
> +  "<html><head><style>" +
> +  "  select { transition: all .1s; }" +
> +  "  select:focus { text-shadow: 0 0 0 #303030; }" +
> +  "</style></head><body><select id='one'>" +
> +  '  <option>{"text-shadow": "none"}</option>' +

Pretty sure this failed because you didn't specify "color" or "backgroundColor" here.

This should have:
{"color": "rgb(0, 0, 0)", "backgroundColor": "rgba(0, 0, 0, 0)", "text-shadow": "none"}

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4615d6279e38
Update transitionend properties of <option> elements on a deferred task. r=jaws

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Sorry, I had that but removed it last minute due to having added the skipSelectColorTest flag. Turns out that flag only skips the tests on the <select> itself, but not the <option>

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4615d6279e38

Comment 13

[Julien Cristau [:jcristau]]

(In reply to :Felipe Gomes (needinfo me!) from comment #4)
> (probably not worth uplifting for 55 since a menulist with 2000 items should
> not be common)

wontfix for 55 per comment 4 and the fact that we're at RC stage.

Comment 14

[David Durst [:ddurst]]

(Marking as qf:p1 because it blocks bug 1118086 which itself is a qf:p1. If 1118086 should be changed to a meta, then let's change it.)