Closed Bug 1383205 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Layout: Form Controls, defect)

54 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Performance Impact high
Tracking Status
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- fixed

People

(Reporter: drno, Assigned: Felipe)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
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.
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.
Assignee: nobody → felipc
Status: NEW → ASSIGNED
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)
Blocks: 1352963, 1118086
No longer depends on: 1118086
Whiteboard: [qf]
(probably not worth uplifting for 55 since a menulist with 2000 items should not be common)
OS: Mac OS X → All
Hardware: x86_64 → All
Version: Trunk → 54 Branch
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)
Attachment #8891354 - Flags: review?(jaws) → review+
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a6100bac239
Update transitionend properties of <option> elements on a deferred task. r=jaws
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
Flags: needinfo?(felipc)
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"}
Pushed by felipc@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4615d6279e38
Update transitionend properties of <option> elements on a deferred task. r=jaws
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>
Flags: needinfo?(felipc)
https://hg.mozilla.org/mozilla-central/rev/4615d6279e38
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(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.
(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.)
Whiteboard: [qf] → [qf:p1]
Blocks: 1391161
Performance Impact: --- → P1
Whiteboard: [qf:p1]
You need to log in before you can comment on or make changes to this bug.