Closed
Bug 1113206
Opened 9 years ago
Closed 9 years ago
<select> elements are flaky in vertical layout
Categories
(Core :: Layout: Form Controls, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: smontagu, Assigned: jfkthame)
References
(Blocks 2 open bugs)
Details
Attachments
(7 files, 2 obsolete files)
1.41 KB,
text/html
|
Details | |
1.33 KB,
text/html
|
Details | |
66.30 KB,
patch
|
Details | Diff | Splinter Review | |
67.34 KB,
patch
|
smontagu
:
review+
|
Details | Diff | Splinter Review |
4.79 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
4.53 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
46.09 KB,
image/png
|
Details |
Set vertical writing mode in the attached test cases Problems include: ###!!! ASSERTION: Computed overflow area must contain frame bounds: 'aNewSize.width == 0 || aNewSize.height == 0 || r->width == nscoord_MAX || r->height == nscoord_MAX || (mState & NS_FRAME_SVG_LAYOUT) || r->Contains(nsRect(nsPoint(0,0), aNewSize))', file /Users/smontagu/mozwork/hgtree/mozilla/layout/generic/nsFrame.cpp, line 7423 ###!!! ASSERTION: We should not have a computed height here!: 'aReflowState.ComputedHeight() == nscoord(1 << 30)', file /Users/smontagu/mozwork/hgtree/mozilla/layout/forms/nsListControlFrame.cpp, line 476 The line block-size of the options is much too wide. The combobox is completely misrendered. The alignment of the list box is inconsistent between vertical-lr and vertical-rl Some of these issues, not all, are fixed by patches which I will be attaching to bug 1079154 once they pass try
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Comment 2•9 years ago
|
||
This fixes the assertions mentioned above, but reintroduces two new ones: ASSERTION: FinishReflowChild with unconstrained container width!: 'aContainerWidth != NS_UNCONSTRAINEDSIZE', file ... layout/generic/nsContainerFrame.cpp, line 961 ASSERTION: FinishReflowChild with unconstrained container width!: 'aContainerWidth != NS_UNCONSTRAINEDSIZE', file ... layout/generic/nsContainerFrame.cpp, line 1104 (in the first one "FinishReflowChild" is a typo or mis-copy-paste for "ReflowChild".
Assignee: nobody → smontagu
Reporter | ||
Comment 3•9 years ago
|
||
The patch also seems to fix the testcase in bug 1112954. Bug 1119475 will make a lot of difference to the rendering here, and maybe the assertions as well. I'll revisit after that is checked in.
Assignee | ||
Comment 4•9 years ago
|
||
I was about to work on something like this, when I remembered you'd got some WIP already. So instead I've just un-bitrotted this, and tested with current trunk code; looking pretty good. I have a couple of additional fixes that I'll add as a followup patch.
Assignee | ||
Comment 5•9 years ago
|
||
With this on top of your main patch here, things seem to work pretty well.
Attachment #8590387 -
Flags: feedback?(smontagu)
Assignee | ||
Updated•9 years ago
|
Attachment #8590384 -
Attachment description: Make nsComboboxControlFrame and nsListControlFrame use inline/block size instead of width/height → Make nsComboboxControlFrame and nsListControlFrame use inline/block size instead of width/height [smontagu's patch, refreshed]
Assignee | ||
Comment 6•9 years ago
|
||
The main remaining issue, AFAICT, is that the native-theme rendering of <select> elements looks terrible when we try to apply it in vertical mode, at least on OS X. That's probably best split off into a separate bug. For now, using -moz-appearance:none on vertical <select>s avoids the worst of the ugliness.
Assignee | ||
Updated•9 years ago
|
Blocks: enable-writing-mode-release
Assignee | ||
Comment 7•9 years ago
|
||
Simon, this is your earlier patch here, rebased and with my followup folded in. I've marked this r=me, but I guess it'd be good for you to review my changes to it before we consider it ready to land. AFAICT, together with a widget patch (coming next), this makes things work pretty well in non-e10s windows. In e10s, we have a further problem because the dropdown for a <select> control gets proxied to the parent process, which then uses a XUL popup to render it, and that doesn't have vertical support. But I think we should make that a followup bug.
Attachment #8613488 -
Flags: review?(smontagu)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8613489 -
Flags: review?(roc)
Assignee | ||
Updated•9 years ago
|
Attachment #8590384 -
Attachment is obsolete: true
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8590387 [details] [diff] [review] patch 2 - Additional fixup for nsComboboxControlFrame and nsListControlFrame Folded this into attachment 8613488 [details] [diff] [review].
Attachment #8590387 -
Flags: feedback?(smontagu)
Assignee | ||
Updated•9 years ago
|
Attachment #8590387 -
Attachment is obsolete: true
Assignee | ||
Comment 10•9 years ago
|
||
And filed bug 1170129 as a followup about the problem with e10s mode.
Reporter | ||
Comment 11•9 years ago
|
||
Comment on attachment 8613488 [details] [diff] [review] Make nsComboboxControlFrame and nsListControlFrame use logical coordinates and support vertical writing modes Review of attachment 8613488 [details] [diff] [review]: ----------------------------------------------------------------- r=me for your fixes
Attachment #8613488 -
Flags: review?(smontagu) → review+
Attachment #8613489 -
Flags: review?(roc) → review+
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c838abaf38a4 https://hg.mozilla.org/integration/mozilla-inbound/rev/c53efb981810
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c838abaf38a4 https://hg.mozilla.org/mozilla-central/rev/c53efb981810
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Assignee | ||
Comment 14•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a603ab448b67
Assignee | ||
Comment 15•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ffe8f328931
Assignee | ||
Comment 16•9 years ago
|
||
The patches here landed with only manual testing; here's a basic mochitest that checks the popup is appearing on the expected (increasing block-direction) side of the element. This won't work under e10s until bug 1170129 is fixed, so skipping it for now.
Attachment #8643633 -
Flags: review?(roc)
Assignee | ||
Updated•9 years ago
|
Assignee: smontagu → jfkthame
Attachment #8643633 -
Flags: review?(roc) → review+
Assignee | ||
Comment 17•9 years ago
|
||
url: https://hg.mozilla.org/integration/mozilla-inbound/rev/8a31c820b381e29e2db38a713e1c4bf421097976 changeset: 8a31c820b381e29e2db38a713e1c4bf421097976 user: Jonathan Kew <jkew@mozilla.com> date: Wed Aug 05 12:43:24 2015 +0100 description: Bug 1113206 followup - Add simple test for <select> elements in vertical writing mode. r=roc
Comment 20•9 years ago
|
||
There is a sample page. http://www.mongolfont.com/test/firefox/select.html It seams not working correctly.
Comment 21•9 years ago
|
||
Siquinbilige, Type in the address bar: about:preferences#general and then uncheck Activate multi-processus mode . Click OK and then Firefox should restart with the same opened tabs. Then try again your own test.
Updated•8 years ago
|
Depends on: CVE-2016-2822
Comment 22•6 years ago
|
||
(In reply to Gérard Talbot from comment #21) > Siquinbilige, > > Type in the address bar: > about:preferences#general > and then uncheck Activate multi-processus mode . Click OK and then Firefox > should restart with the same opened tabs. Then try again your own test. I did not able to found setting item multi-processus on mode on 57.0.2. How to set it to select working correctly in vertical writing mode.
Comment 23•6 years ago
|
||
Siquinbilige, Thank you for reporting this. I am using Firefox 59.0a1 buildID=20171211105838 and the expanded parts of selects are no longer displayed according to their associated writing-mode in these tests (and others too) http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/wm-form-controls-sideways-lr-Example-4.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/wm-form-controls-sideways-rl-Example-4.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/wm-form-controls-vertical-lr-Example-4.xht http://www.gtalbot.org/BrowserBugsSection/CSS3WritingModes/wm-form-controls-vertical-rl-Example-4.xht . And multi-process is supposed to be ON, to be enabled by default in 57.0.2 (and in 59.0a1). So, right now, I do not have an answer to your question. I looked at the support files on/about multi-process and read https://support.mozilla.org/en-US/questions/1170439 but I have browser.tabs.remote.autostart and browser.tabs.remote.autostart.2 already set to true. Right now, I believe we should create a new bug report on this, after searching bug reports on multi-process (electrolysis) and select. Siquinbilige, I am using Linux 4.9.0-4-amd64 Debian 9.3 (codename stretch). What is your operating system?
Comment 24•6 years ago
|
||
Siquinbilige, I see that you have joined already bug 1170129 . So, there is not much we can do now.
Comment 25•6 years ago
|
||
I am using Windows 7 Ultimate service pack 1.
Comment 26•6 years ago
|
||
It was worked when I have browser.tabs.remote.autostart and browser.tabs.remote.autostart.2 already set to false. Thank you.
Comment 27•6 years ago
|
||
It was worked when I have browser.tabs.remote.autostart and browser.tabs.remote.autostart.2 set to false. Thank you.
You need to log in
before you can comment on or make changes to this bug.
Description
•