Opened 10 years ago

Closed 7 years ago

#9449 closed defect (fixed)

test_BorderContainer: unexpected scrollbars in Safari 4

Reported by: haysmark Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.1
Keywords: Cc:
Blocked By: Blocking:

Description

In test_BorderContainer, go down to the very simply styled "More fun with layouts" BorderContainer?. If you move the leftmost vertically aligned spacer just a little, all of the sudden all of the BorderContainers? are filled with scrollbars, even if you move the bar back to its original location. Observed on Safari 4 on Windows but haven't tested Mac.

Attachments (3)

test_BorderContainer.html (1.9 KB) - added by bill 10 years ago.
simplified test case
Screen shot 2009-10-05 at 11.15.18 AM.png (26.0 KB) - added by Nathan Toone 10 years ago.
Padding issue
Screen shot 2009-10-05 at 11.19.05 AM.png (24.8 KB) - added by Nathan Toone 10 years ago.
Scroll bars without padding issue (from revision [20078])

Download all attachments as: .zip

Change History (27)

comment:1 Changed 10 years ago by Adam Peller

Milestone: tbd1.4

comment:2 Changed 10 years ago by PhilippC

happens in Chrome, too (V2.0 on Windows Vista)

comment:3 Changed 10 years ago by bill

Owner: changed from Adam Peller to bill
Status: newassigned

Doesn't happen on chrome for me, just safari.

I think it's to do with those intermediate ContentPane's that contain a single BorderContainer child. I'm seeing the ContentPane with width=150 but it's child BorderContainer with width=160.

comment:4 Changed 10 years ago by bill

(In [20069]) add id's for easier debugging, refs #9449 !strict

Changed 10 years ago by bill

Attachment: test_BorderContainer.html added

simplified test case

comment:5 Changed 10 years ago by bill

(In [20078]) If resultSize (second argument) is passed to resize(), don't query the node for it's size. This is better for performance and stability. Similar to the way _LayoutWidget.js works. Refs #9449 !strict.

comment:6 Changed 10 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [20079]) Use overflow:hidden for ContentPanes? w/a single layout child, to avoid safari bugs.

Fixes #9449 !strict.

comment:7 Changed 10 years ago by bill

(In [20080]) Oops, this is one time when we need to explicitly pass false instead of passing undefined.

Refs #9449 !strict.

comment:8 Changed 10 years ago by Nathan Toone

(In [20086]) Refs #9449 - the margin box should be set on the domNode - that is what we were setting it on before. !strict

comment:9 Changed 10 years ago by bill

(In [20353]) Optimized computation of containerNode contentBox size will only work when this.containerNode == this.domNode.

Refs #9449 !strict.

comment:10 Changed 10 years ago by bill

(In [20359]) Fix typo, refs #9449, fixes #9999 !strict.

comment:11 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: closedreopened

The fix (setting overflow:hidden) in [20079] causes breakage in the i18n demo.

The problem is that scrollbars no longer appear on a ContentPane? within a BorderContainer? when it has a single child widget (like the tree on the left-hand side).

comment:12 Changed 10 years ago by bill

The problem is that scrollbars no longer appear on a ContentPane within a BorderContainer when it has a single child widget (like the tree on the left-hand side).

Basically that's correct behavior (and the intention of the patch). When ContentPane contains a single layout widget as it's child, the ContentPane shouldn't display a scrollbar.... the idea being that the underlying widget itself will supply it's own scrollbar (or it will do something so that there's no need for a scrollbar).

The actual test that ContentPane does is whether the underlying widget has a resize() method. Tree breaks this pattern since it has a resize() method but that resize() method doesn't even take a size parameter.

comment:13 Changed 10 years ago by bill

PS: when I wrote "layout widget" above I should have actually wrote something like "sizable widget". A ContentPane will size an Editor or Grid equally as well as a TabContainer, BorderContainer, etc. Anything that can be a child of a layout container can function as a "single child" of ContentPane.

comment:14 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [20385]) Make Tree behave properly as the child of a layout container. Fixes #9449 !strict.

comment:15 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: closedreopened

It's still not quite there...the scrollbars now appear for each "sub-tree" on the same example (http://archive.dojotoolkit.org/nightly/checkout/demos/i18n/demo.html)

To see open make the left-hand pane narrow enough that when you open Antarctica the left/right scrollbar appears. Scrolling this *ONLY* scrolls the "Antarctica" block. This gets really confusing if you open two different blocks (i.e. North America and Antarctica) where they *each* have their own horizontal scroll bars, and they each scroll separately.

This is a completely different from how it behaved before these changes.

comment:16 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [20408]) Only meant to apply the overflow: auto on the tree root, not on every branch. Fixes #9449 !strict.

Changed 10 years ago by Nathan Toone

Padding issue

comment:17 Changed 10 years ago by Nathan Toone

I'm still not quite sold on this solution either...(though probably not enough to reopen the bug...). Previously, the scroll bars would show up as part of the content pane...now, they show up as part of the child...which means that there is, by default, a 5px padding between the scroll bars and the content pane. (see the previously attached screenshot)

I just think it "looks" a bit sloppier than having the scroll bars as a part of the content pane.

Changed 10 years ago by Nathan Toone

Scroll bars without padding issue (from revision [20078])

comment:18 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: closedreopened

I'm also seeing issues (still) with a tree inside an accordion pane as well. It doesn't get resized to the correct height.

comment:19 Changed 10 years ago by bill

Previously, the scroll bars would show up as part of the content pane...now, they show up as part of the child...which means that there is, by default, a 5px padding between the scroll bars and the content pane.

Hi, I agree that the padding is weird, but I think it's a larger issue. I think when there's a single (layout widget) child there shouldn't be any padding used... the ContentPane should be invisible, just a layer for deferring loading.

See for example test_TabContainer.html and compare the "Inlined SubTabContainer" tab to the "SubTabcontainer from href" tab. They look they same, as they should. That's currently implemented via a style="padding: 0" in the test file though, would be better to have it be the default when there's a single child.

I'm also seeing issues (still) with a tree inside an accordion pane as well. It doesn't get resized to the correct height.

Sounds like a serious problem... test case?

comment:20 Changed 10 years ago by bill

I filed #10030 to keep track of that padding issue.

comment:21 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: reopenedclosed

It's not an accordionpane...it's a custom widget that we were using...not a problem with this change.

comment:22 Changed 10 years ago by bill

(In [20532]) Only do the overflow: auto setting when Tree is functioning as a layout widget (ie, when resize(changeSize) (with a changeSize) has been called. Otherwise do the old overflow: visible.

I think this will be a less breaking change, particularly for the case where a narrow <div> contains a Tree widget, in that a horizontal scrollbar won't show up directly underneath the

Refs #9449, #10076 !strict.

comment:23 Changed 7 years ago by adros

Resolution: fixed
Status: closedreopened

If dijit.form.Form is the only child of a ContentPane?, the contentPane has class "dijitContentPaneSingleChild", so it has no scrollbars. But there are neither no scrollbars in the form.

comment:24 Changed 7 years ago by bill

Resolution: fixed
Status: reopenedclosed

I don't understand what you are saying, "neither no" in the sentence above isn't proper english. But in any case, if you are having a problem you'll need to open a new ticket and attach a test case. All tickets need test cases.

Note: See TracTickets for help on using tickets.