Closed Bug 344561 Opened 18 years ago Closed 10 years ago

Day/Week view: Header boxes are misaligned if scrollbars are shown

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ssitter, Assigned: bv1578)

References

Details

Attachments

(12 files, 2 obsolete files)

Fallout from Bug 323093:
Day/Week view: Header boxes are misaligned if scrollbars are shown

Steps to Reproduce:
1. Start Sunbird with clean profile
2. Switch to Day view or week view, minimize window to get scrollbars
3. Check view layout

Actual Results:
Header boxes are misaligned / have wrong size compared to event boxes below.

Expected Results:
Header boxes have the same wide as the event boxes below.

Additional Information:
It seems to work fine after pressing 'Rotate' button twice.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060713 Sunbird/0.3a2+
Attached image screenshot
Not to jack your bug, but why is everything EXCEPT the minimonth in en-US?!?
(In reply to comment #2)
> Not to jack your bug, but why is everything EXCEPT the minimonth in en-US?!?

Good question, but that's definitely a separate issue.
Attached patch patch v1 (obsolete) — Splinter Review
I think it makes sense to not rely on any modified events for the
scrollbar disabled attribute, when the view is layouted.
Therefore I moved the adjustment of the scrollbar spacers from
the reorient to the relayout method.
Assignee: base → thomas.benisch
Status: NEW → ASSIGNED
Attachment #229818 - Flags: first-review?(jminta)
(In reply to comment #3)
> (In reply to comment #2)
> > Not to jack your bug, but why is everything EXCEPT the minimonth in en-US?!?
> 
> Good question, but that's definitely a separate issue.
> 
I got the answer in IRC. Thanks.
Comment on attachment 229818 [details] [diff] [review]
patch v1

everyone seems to put a pretty high priority on view performance.  Is there  a fix for this that won't actually be such a hit?  Can we have an early return flag or something for cases where we know this calculation is silly?
(In reply to comment #6)
> (From update of attachment 229818 [details] [diff] [review] [edit])
> everyone seems to put a pretty high priority on view performance.  Is there  a
> fix for this that won't actually be such a hit?  Can we have an early return
> flag or something for cases where we know this calculation is silly?

I think the relayout method is semantically the right place for
adjusting the scrollbar spacers. Whenever the view boxes are
relayouted it makes sense, that also the scrollbar spacers are
adjusted. Probably one finds another place after a refresh() call 
for fixing exactly this bug, but then it's just a question of time 
until the next bug shows up.

Apart from that I have no idea how to determine in the 
adjustScrollBarSpacers method that this calculation is unnecessary.
The only idea I have would be to store the last setted spacer size
in a field m_ScrollBarSpacerSize and before setting the attribute
the next time to compare propertyValue with m_ScrollBarSpacerSize.
I personally don't like this approach, probably you have any
other ideas?
Comment on attachment 229818 [details] [diff] [review]
patch v1

The only real problem here is that on construction, there's nothing to force the views to scroll.  Once we actually force 24hrs to be shown, we know apriori that there will be.  So, let's land this for now, with a big XXX comment that this change should be removed once we have stuff in the columns.
Attachment #229818 - Flags: first-review?(jminta) → first-review+
(In reply to comment #8)
> (From update of attachment 229818 [details] [diff] [review] [edit])
> The only real problem here is that on construction, there's nothing to force
> the views to scroll.  Once we actually force 24hrs to be shown, we know apriori
> that there will be.  So, let's land this for now, with a big XXX comment that
> this change should be removed once we have stuff in the columns.

I think I still didn't get your point. When the view boxes are constructed with
a scrollbar at the childbox initially enabled, but no disabled modified event is
fired, then the size of the scrollbar spacers must be set also initially.
This will be also the case for 24 hours. Therefore I don't understand, why
the adjustScrollBarSpacers call can be removed from the relayout method after
24 hours view is enabled.

(In reply to comment #9)
> (In reply to comment #8)
> I think I still didn't get your point. When the view boxes are constructed with
> a scrollbar at the childbox initially enabled, but no disabled modified event
> is
> fired, then the size of the scrollbar spacers must be set also initially.
So, if the views are statically contructed, then setting the scrollbar spacers sizers in the <constructor> will give us the right numbers.  Right now, when we construct the view, there are no scrollbars, so we get 0, instead of the number we want.

My point is that later, we can just call this function in the constructor and on rotate, and not in the normal relayout.  Or am I missing something?

Attached patch patch v2Splinter Review
Added a XXX comment as requested in comment #8.

I also added the r+ flag from the last patch.
Attachment #229818 - Attachment is obsolete: true
Attachment #230558 - Flags: first-review+
(In reply to comment #10)
I think I now got it. I don't have any scenario in mind, where this fails,
but anyway this needs intensive testing.

Can you please land this patch.
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
This is not fully fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060811 Calendar/0.3a2+

There are three scenarios:

1. If I start Sunbird in Day/Multiweek/Month view and switch to the week view, 
   everything is fine --> bug is fixed
2. If I start Sunbird in Week view and am subscribed to a remote ics calendar, 
   Sunbird still shows this bug until the remote calendar is fully loaded. 
   After it has been loaded the bug disappears. --> bug is partly fixed
3. If I start Sunbird in week view and only have events in my home calendar 
   (storage provider), Sunbird still shows this bug. --> bug is not fixed
Status: RESOLVED → REOPENED
Component: Internal Components → Calendar Views
QA Contact: base → views
Resolution: FIXED → ---
The problem is, that the calendar-multiday-view binding doesn't get
any DOMAttrModified events for the disabled attribute of the
scrollbar. This seems to be a trunk only problem, because on the
MOZILLA_1_8_BRANCH the view gets those events.

Any help on this problem is appreciated.
I'd suggest filing a bug on that if there isn't one already and marking this bug as dependent on that one.  CCing views@calendar.bugs on that bug seems like a reasonable thing.
(In reply to comment #16)
> I'd suggest filing a bug on that if there isn't one already and marking this
> bug as dependent on that one.  CCing views@calendar.bugs on that bug seems like
> a reasonable thing.

I filed a bug for the missing DOMAttrModified events (see #349503).
Probably this is not a bug, but was changed on trunk by intention.
We will see.

Nevertheless I found a way to fix this bug. Therefore I didn't set a dependency.
This patch fixes the problem of the misaligned header boxes on trunk.

Instead of relying on the DOMAttrModified event for the disabled attribute of
the scrollbar I now use the underflow and overflow events. This works on both,
the MOZILLA_1_8_BRANCH and on trunk, for Lightning and for SunBird.

As this bug is really annoying for any SunBird user I propose to land this
patch for the 0.3 release.
Attachment #234763 - Flags: first-review?(jminta)
Comment on attachment 234763 [details] [diff] [review]
misaligned header boxes on trunk

r=jminta, with a line-wrap on the scrollbox line.
Attachment #234763 - Flags: first-review?(jminta) → first-review+
misaligned header boxes on trunk patch checked in.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Looks correct for me. 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060821 Calendar/0.3a2+
Status: RESOLVED → VERIFIED
Would like to reopen this bug - having a similar bug when rotating day/week view - Linux build 2006082206

Screenshots to follow
Attached image Misaligned Rotated View
(In reply to comment #22)
> Would like to reopen this bug - having a similar bug when rotating day/week
> view - Linux build 2006082206
> 
> Screenshots to follow
> 

Have discussed this in #calendar-qa. Looks like it might be a linux regression on this same issue. Reopening.

Status: VERIFIED → REOPENED
Resolution: FIXED → ---
This patch fixes the misaligned rotated view problem.

The problem is that the time string, e.g. "09:00 AM" determines the
size of the time boxes. In this case the time boxes are wider than
the calendar-event-column-lineboxes, and therefore the whole view
looks misaligned.

This patch sets the "overflow: hidden" style for the time boxes, so
that the size is determined by the value of pixelsPerMinute instead
of the time string. In addition, the value for
mMinHorizontalPixelsPerMinute is increased from 1.0 to 1.2, because
otherwise a long time string "09:00 AM" is clipped.

tbe->jminta:
Please feel free to choose another value for
mMinHorizontalPixelsPerMinute. If the value is too small, the
time string is clipped, but the grid is still aligned.
Attachment #235078 - Flags: first-review?(jminta)
Comment on attachment 235078 [details] [diff] [review]
misaligned rotated view patch

r=jminta
Attachment #235078 - Flags: first-review?(jminta) → first-review+
Patch checked in, with a fix for dmose's cross-commit error earlier in the week.
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
I had this problem when I resize unifinder but right now it is ok

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060823 Calendar/0.3a2+
Status: RESOLVED → VERIFIED
*** Bug 355876 has been marked as a duplicate of this bug. ***
Reopening based on dupe and my own testing, both on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061006 Sunbird/0.3

See http://members.lycos.co.uk/hipermetropia/bugs/2006-10-07/image.jpeg

My own results show the although not with so drastic differences. Seems to be a rounding issue.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee: thomas.benisch → michael.buettner
Status: REOPENED → NEW
Is anybody still seeing this issue using Sunbird 0.5 or Lightning 0.5?
I can still confirm this problem with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6pre) Gecko/20070726 Calendar/0.7pre, when I startup Sunbird in week view    with and also without scrollbar.
Assignee: michael.buettner → nobody
It may be a bit hard to reproduce, but I still get this problem using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7pre) Gecko/20070807 Calendar/0.7pre. Here go a couple of screen-shots. It happens when I'm viewing a certain week and then navigate to another week that needs /more space/ to show the all-day tasks.
http://members.lycos.co.uk/hipermetropia/bugs/2007-08-08/1.jpeg
http://members.lycos.co.uk/hipermetropia/bugs/2007-08-08/2.jpeg
OS: Windows 2000 → All
Hardware: PC → All
Flags: wanted-calendar0.8?
Version: Trunk → unspecified
Still in the actual 0.7 release. :[
(build 2007102304)
Setting wanted0.8- as the main Calendar developers will not devote any time to
this in the 0.8 timeframe. Patches are, of course, always welcome.
Severity: normal → minor
Flags: wanted-calendar0.8? → wanted-calendar0.8-
Attached image before
Still occurs in mac branch 20080220 Sunbird build, fresh profile.

Note the red arrow, when Sunbird just launches in online state by default.
Attached image after
Once switching to offline state by clicking on the bottom left bulb icon, the alignment "jumps" back.
Found in 0.7 release, screenshot attached. I start off in week view. Clicking on Today fixes the skew.
Flags: wanted-calendar0.8- → wanted-calendar0.9+
Moving the call
"this.adjustScrollBarSpacers();"
from "relayout()" to  "resize()" seems to fix the problem also for Sunbird but  results in a performance loss. On start up this is called 10 times (in Lightning) compared to 2 times when the call is placed in "relayout()".
Flags: wanted-calendar0.9+ → wanted-calendar0.9-
isn't it already fixed in 0.9?
(In reply to comment #47)
> isn't it already fixed in 0.9?

Haven't seen this in 0.9 after the calendar view redesign. -> Resolving as WFM.

Stefan, can you confirm this?
Status: NEW → RESOLVED
Closed: 18 years ago16 years ago
Resolution: --- → WORKSFORME
I've updated to 0.9 and the bug is not solved for me. The rows are misaligned...

When the "search for ..." toolbar is hidden the the calender view is OK.
(In reply to comment #49)
> I've updated to 0.9 and the bug is not solved for me. The rows are
> misaligned...
> 
> When the "search for ..." toolbar is hidden the the calender view is OK.

Sorry, you are right. -> REOPENING
Status: RESOLVED → REOPENED
Flags: wanted-calendar0.9- → wanted-calendar1.0?
Resolution: WORKSFORME → ---
Status: REOPENED → NEW
i see this issue using one of the latest sunbird builds: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090209 Calendar/1.0pre

1. i open sunbird. i see alignment issues
2. i go to the next calenderweek and back to the current calender week
3. the alignment issues are gone.

is this the same bug or a different one? cheers,
Attached image alignment_issues_1.png
Attached image alignment_issues_2.png
I see this bug in 1.0 under Mac OS X
Is this a Sunbird Only issue maybe? I couldn't reproduce this with Lightning.
(In reply to comment #56)
> Is this a Sunbird Only issue maybe? I couldn't reproduce this with Lightning.

No, I'll attach a screenshot of Lightning later:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.8) Gecko/20100216 Lightning/1.0b2pre Thunderbird/3.0.2
version: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.8) Gecko/20100216 Lightning/1.0b2pre Thunderbird/3.0.2
Today I tried to downgrade Lightning from nightly to 1.0b1, the problem remains. Then I uninstalled Lightning, delete all files (storage.sdb, calendar-data/, and all calender.* in prefs.js), reinstall Lightning 1.0b1, readd my calendars (they're all in the cloud, luckily), and it seems to be OK now. It's already hours and I didn't see this bug again.
(In reply to comment #56)
> Is this a Sunbird Only issue maybe? I couldn't reproduce this with Lightning.

(In reply to comment #59)
> ..., and it seems to be OK now. It's already hours and I didn't see this bug again.

Could you try these steps to reproduce:

1. in week view create an event from 23:45 to 0:45 of the following day (if you have a monitor with a very high resolution set the start to 23:59);
2. zoom out the view (press "Ctrl" + "-" keys) until the full day is showed (24 complete hours);

now the bug should appear.
(In reply to comment #60)
> (In reply to comment #56)
> > Is this a Sunbird Only issue maybe? I couldn't reproduce this with Lightning.
> 
> (In reply to comment #59)
> > ..., and it seems to be OK now. It's already hours and I didn't see this bug again.
> 
> Could you try these steps to reproduce:
> 
> 1. in week view create an event from 23:45 to 0:45 of the following day (if you
> have a monitor with a very high resolution set the start to 23:59);
> 2. zoom out the view (press "Ctrl" + "-" keys) until the full day is showed (24
> complete hours);
> 
> now the bug should appear.

well, I tried it and it can't reproduce this bug for me. but the block appears in a wrong position (I crated a event from 23:55~0:55, but it appears at 0:55~1:55)
Flags: wanted-calendar1.0?
WFM. Anyone still seeing this problem in recent Lightning releases, e.g. Lightning 1.0 (Thunderbird 8) or newer?
(In reply to Stefan Sitter from comment #62)
> WFM. Anyone still seeing this problem in recent Lightning releases, e.g.
> Lightning 1.0 (Thunderbird 8) or newer?
I haven't seen it for a while now, never on Lightning 1.0.
I can still see this bug with steps to reproduce mentioned in comment #60 (Lightning 1.4a2 Thunderbird 12.0a2).
Alternatively you can first zoom the week view so that all the 24 hours are showed, then create the event with start time at 23:50 and 1 hour duration. The event's part in the starting day needs more than the equivalent of 10 minutes as vertical space in order to show the text in the box, so the scroll bar appears, the view resizes the width, but the view header doesn't resize.

For me the first part of the patch "misaligned header boxes on trunk" https://bugzilla.mozilla.org/attachment.cgi?id=234763 fixes the issue, but it would need more testing.

This is the patch ported on comm-central:

diff --git a/calendar/base/content/calendar-multiday-view.xml b/calendar/base/content/calendar-multiday-view.xml
--- a/calendar/base/content/calendar-multiday-view.xml
+++ b/calendar/base/content/calendar-multiday-view.xml
@@ -2429,8 +2429,8 @@
 <xul:box anonid="headertimespacer" class="multiday-view-header-time-spacer"/>
           <xul:box anonid="headerdaybox" class="multiday-view-header-day-box" flex="1" equalsize="always" />
           <xul:box anonid="headerscrollbarspacer"/>
         </xul:box>
-        <xul:scrollbox anonid="scrollbox" flex="1">
+        <xul:scrollbox anonid="scrollbox" flex="1" onoverflow="adjustScrollBarSpacers();" onunderflow="adjustScrollBarSpacers();">
           <!-- the orient of the calendar-time-bar needs to be the opposite of the parent -->
           <xul:calendar-time-bar xbl:inherits="orient" anonid="timebar"/>
           <xul:box anonid="daybox" class="multiday-view-day-box" flex="1"
(In reply to Decathlon from comment #64)
> I can still see this bug with steps to reproduce mentioned in comment #60
> (Lightning 1.4a2 Thunderbird 12.0a2).
> Alternatively you can first zoom the week view so that all the 24 hours are
> showed, then create the event with start time at 23:50 and 1 hour duration.
> The event's part in the starting day needs more than the equivalent of 10
> minutes as vertical space in order to show the text in the box, so the
> scroll bar appears, the view resizes the width, but the view header doesn't
> resize.

i can confirm this on ubuntu 11.10 oneiric running
thunderbird 11.0~b4+build1-0ubuntu0.11.10.1~mtn1 and 
lightning 1.3~b1+build1-0ubuntu0.11.10.1~mtn1

i haven't tested the fix.
Attached patch overflow-underflow-patch-v1 (obsolete) — Splinter Review
This bug is always reproducible according to steps to reproduce in comment 60 (eventually invert step 1 with step 2), moreover the union of the fix for Bug 872063 and Bug 998260 now make this bug always visible in week view (with the latest nightly Build ID 20140506030207) and not only in the particular case described in comment 60.

The patch reported in comment 64 seems working fine for the this bug and for the issue caused by the bugs mentioned above. Eventually it needs to be tested on other platform than Windows.
Assignee: nobody → bv1578
Status: NEW → ASSIGNED
Attachment #8417996 - Flags: review?(philipp)
Does this work on the edge case? I recall there was a possibility of an endless loop where onoverflow/underflow methods were needed. Lets say the overflow occurs, the handler decreases the container width. As a result, the underflow handler is called and increases the container width. This in turn would cause an overflow again.

I could not reproduce the bug at all on mac, but thats ok because mac hides the spacers anyway due to the overlay scrollbars. I assume this is similar on gnome with overlay scrollbars.
(In reply to Philipp Kewisch [:Fallen] from comment #67)
> Does this work on the edge case? I recall there was a possibility of an
> endless loop where onoverflow/underflow methods were needed. 

Not sure to understand correctly.
Is there a Geko's bug about onunderflow onoverflow such as a loop could be triggered or this could be caused by a not proper code? 
Normal workflow should be such as: overflow occurs, then you use onoverflow to trigger a code that will adapt a new size for the container box (or for the box that overflowed) and eventually use onunderflow to see when your new size is correct.
The code in the patch doesn't work like that, onoverflow/onunderflow call adjustScrollBarSpacers() which sets the width of the spacers in other boxes outside the one where overflow/underflow occurs (the elements with anonid "headerscrollbarspacer" and "labelscrollbarspacer"), hence the size of the scrollbox don't change because of adjustScrollBarSpacers() and overflow/underflow are not caused by the code in the patch. It doesn't seem that a loop might be triggered in this way.

From what I can verify, with the steps to reproduce in comment 60, no underflow happens afterwards an overflow or vice-versa.

We have another approach with onoverflow/onunderflow in the calendar-list-tree (though a less complex case):
http://mxr.mozilla.org/comm-central/source/calendar/base/content/widgets/calendar-list-tree.xml#215


> 
> I could not reproduce the bug at all on mac, but thats ok because mac hides
> the spacers anyway due to the overlay scrollbars. I assume this is similar
> on gnome with overlay scrollbars.

About reproducibility, I have to say that according to STR in comment 60 the bug is fully reproducible on Windows 7.
I admit it's a particular case, but the need for a fix is caused by the misaligned that now is *always* visible in week view (when Thunderbird restart for the second time) because of bug 998260 and the fix for bug 872063.
Now the week view is always misaligned (on Windows 7) unless a refresh occurs and this is by far a less particular case.
Do you think it's better to open a specific bug for this case and find a specific solution instead of using a fix for this bug?
Comment on attachment 8417996 [details] [diff] [review]
overflow-underflow-patch-v1

Review of attachment 8417996 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Decathlon from comment #68)
> (In reply to Philipp Kewisch [:Fallen] from comment #67)
> > Does this work on the edge case? I recall there was a possibility of an
> > endless loop where onoverflow/underflow methods were needed. 
> 
> Not sure to understand correctly.
> Is there a Geko's bug about onunderflow onoverflow such as a loop could be
> triggered or this could be caused by a not proper code? 
> Normal workflow should be such as: overflow occurs, then you use onoverflow
> to trigger a code that will adapt a new size for the container box (or for
> the box that overflowed) and eventually use onunderflow to see when your new
> size is correct.
Not a bug and given you clarified that the spacer is changed in another box your code is fine. I was thinking about a different case where: overflow occurs, handler reduces size of elements inside the overflowing box. This causes an underflow to occur, so the handler increases size of those elements again, which causes an overflow to occur.... I initially had this problem when working on the patch to use the long/short weekday name in the day header and just wanted to make sure we don't have a similar situation here.

> > I could not reproduce the bug at all on mac, but thats ok because mac hides
> > the spacers anyway due to the overlay scrollbars. I assume this is similar
> > on gnome with overlay scrollbars.
> 
> About reproducibility, I have to say that according to STR in comment 60 the
> bug is fully reproducible on Windows 7.
Oh I wasn't saying it shouldn't be fixed, just that I couldn't test it because it doesn't show on mac. Your fix is perfectly fine!

r=philipp
Attachment #8417996 - Flags: review?(philipp) → review+
Patch overflow-underflow-patch-v1 with data for checkin.

r=philipp
Attachment #8417996 - Attachment is obsolete: true
Attachment #8446411 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/b78e8c3eac69
Status: ASSIGNED → RESOLVED
Closed: 16 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 3.5
You need to log in before you can comment on or make changes to this bug.