Last Comment Bug 391740 - listheader in richlistbox appears to right/at bottom of richlistitems
: listheader in richlistbox appears to right/at bottom of richlistitems
Status: VERIFIED FIXED
:
Product: Toolkit
Classification: Components
Component: XUL Widgets (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla1.9beta1
Assigned To: Philip Chee
:
: Neil Deakin
Mentors:
: 395180 399012 (view as bug list)
Depends on:
Blocks: 377784 396545
  Show dependency treegraph
 
Reported: 2007-08-10 14:03 PDT by Myk Melez [:myk] [@mykmelez]
Modified: 2009-06-10 10:11 PDT (History)
21 users (show)
benjamin: blocking1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case that demonstrates the problem (850 bytes, application/vnd.mozilla.xul+xml)
2007-08-10 14:03 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details
Patch (994 bytes, patch)
2007-09-08 03:03 PDT, Michael Ventnor
asaf: review+
bzbarsky: approval1.9+
Details | Diff | Splinter Review
screenshot (83.29 KB, image/jpeg)
2007-09-11 05:06 PDT, pal-moz
no flags Details
screenshot (76.61 KB, image/jpeg)
2007-09-12 03:56 PDT, pal-moz
no flags Details
screen shot (89.33 KB, image/jpeg)
2007-09-17 21:27 PDT, pal-moz
no flags Details
Proposed patch (1.05 KB, patch)
2007-09-21 10:00 PDT, Philip Chee
asaf: review+
Details | Diff | Splinter Review

Description User image Myk Melez [:myk] [@mykmelez] 2007-08-10 14:03:56 PDT
Created attachment 276173 [details]
test case that demonstrates the problem

In bug 376815, mano added support for using listheader in richlistboxes.  But when I try it, listheader appears to the right of the richlistitems in the richlistbox, not on top, as I would expect.

And orienting the richlistbox vertically also doesn't fix the problem.  It causes the listheader to appear at the bottom, not at the top.

Here's a testcase that demonstrates the problem.  Might this be fixed by bug 367843?
Comment 1 User image Steve England [:stevee] 2007-09-06 04:47:53 PDT
*** Bug 395180 has been marked as a duplicate of this bug. ***
Comment 2 User image Michael Ventnor 2007-09-08 03:03:43 PDT
Created attachment 280161 [details] [diff] [review]
Patch

Found the cause.
Comment 3 User image Hideo Oshima 2007-09-10 06:33:39 PDT
(In reply to comment #2)
> Created an attachment (id=280161) [details]
> Patch
> 
> Found the cause.
 
I tried this patch.
It looks good.
Comment 4 User image Mano (::mano, needinfo? for any questions; not reading general bugmail) 2007-09-10 12:59:49 PDT
Comment on attachment 280161 [details] [diff] [review]
Patch

r=mano
Comment 5 User image Michael Ventnor 2007-09-10 14:54:11 PDT
Comment on attachment 280161 [details] [diff] [review]
Patch

Let's try for M8 approval. My reasoning is that this will at least prevent the inevitable bug reports from M8 users about this issue, and the fix is one line long.
Comment 6 User image Hideo Oshima 2007-09-11 05:01:40 PDT
I found there were two more problems.
1. Scroll with column header. 
   Column header should stay the position.
2. The height of the last row is too narrow.

Should I open new bug?
Comment 7 User image pal-moz 2007-09-11 05:06:13 PDT
Created attachment 280452 [details]
screenshot

with below tip in userChrome.css

#handlersView {
-moz-box-direction: reverse;
}

no problem with scroll, header stays at the top.
but last line is cut off.
Comment 8 User image pal-moz 2007-09-12 03:56:10 PDT
Created attachment 280582 [details]
screenshot

another tip,

#handlersView > listheader {
-moz-box-ordinal-group: 1;
}
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2007-09-16 12:46:28 PDT
Comment on attachment 280161 [details] [diff] [review]
Patch

a=bzbarsky
Comment 10 User image Myk Melez [:myk] [@mykmelez] 2007-09-17 10:59:09 PDT
Thanks for the patch!  I have checked it in.

Checking in toolkit/content/widgets/richlistbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/richlistbox.xml,v  <--  richlistbox.xml
new revision: 1.44; previous revision: 1.43
done


(In reply to comment #6)
> I found there were two more problems.
> 1. Scroll with column header. 
>    Column header should stay the position.
> 2. The height of the last row is too narrow.
> 
> Should I open new bug?

Yes, please do open new bugs for these issues.
Comment 11 User image pal-moz 2007-09-17 21:27:41 PDT
Created attachment 281272 [details]
screen shot

> 2. The height of the last row is too narrow.

no problem with Win XP default theme.
Comment 12 User image Hideo Oshima 2007-09-18 05:34:58 PDT
(In reply to comment #11)
> Created an attachment (id=281272) [details]
> screen shot
> 
> > 2. The height of the last row is too narrow.
> 
> no problem with Win XP default theme.
 
Worksforme too with today's build.
Sorry I don't know why.

Anyway I opened bug 396545 for first problem.
Comment 13 User image Steve England [:stevee] 2007-09-18 08:51:41 PDT
Note: Some people are reporting that the headers are still appearing at the bottom of the list. Littlemutt has discovered that this happens if you have Console² extension installed. Just a headsup.
Comment 14 User image Philip Chee 2007-09-18 09:23:55 PDT
I plan to update Console² in the next few days.
Comment 15 User image Wesley Johnston (:wesj) 2007-09-19 15:10:16 PDT
I don't think this patch really fixed this problem. Adding xul:listheader to the includes statement just makes it NOT match the listheaders (I don't think xbl bindings are that smart about namespaces... or anything really except nodeNames), so they're pushed into the box and Bug 396545 happens.

The reason they're being put at the bottom of the box is because the single listheader element being used here contains some treecols, which get displayed strangely because of the way the tree box model fits things.

An easier fix than that is to just make the richlistbox take a listhead element instead of a listheader element, and then put two listheaders inside of it. Doing so here seemed to make everything work and look fine (minus some theme problems), but I don't have the means to write a diff, and I'm not sure if the listheader/treecol decisions were made for some better reason.
Comment 16 User image Myk Melez [:myk] [@mykmelez] 2007-09-20 11:40:30 PDT
Sounds like this needs a better fix.
Comment 17 User image Dão Gottwald [:dao] 2007-09-21 07:54:09 PDT
In fact, the fix should be backed out.
Comment 18 User image Philip Chee 2007-09-21 09:42:07 PDT
http://lxr.mozilla.org/seamonkey/source/toolkit/content/xul.css#445
listheader {
  -moz-binding: url("chrome://global/content/bindings/listbox.xml#listheader");
  -moz-box-ordinal-group: 2147483646;
}
This was introduced in Bug 178604 which is why the listheader appear at the bottom.
Comment 19 User image Philip Chee 2007-09-21 10:00:10 PDT
Created attachment 281843 [details] [diff] [review]
Proposed patch

This patch fixes the test case in this bug and it doesn't cause the listheader in a richlistbox to scroll, but it might affect consumers of <richlistbox> that assume that the default orientation is horizontal.
Comment 20 User image Philip Chee 2007-09-21 12:59:56 PDT
Comment on attachment 281843 [details] [diff] [review]
Proposed patch

> but it might affect consumers of <richlistbox> that
> assume that the default orientation is horizontal.

Actually prior to the listheader support, all children ended up in a vertical scrollbox anyway so I don't think that there are any usages that depend on a richlistbox being horizontally inclined.
Comment 21 User image Philip Chee 2007-09-21 13:02:17 PDT
Um I forgot to mention that the first patch has to be backed out first.
Comment 22 User image Mano (::mano, needinfo? for any questions; not reading general bugmail) 2007-09-27 12:39:42 PDT
Comment on attachment 281843 [details] [diff] [review]
Proposed patch

r=mano, many thanks.
Comment 23 User image Myk Melez [:myk] [@mykmelez] 2007-09-27 16:06:44 PDT
Comment on attachment 281843 [details] [diff] [review]
Proposed patch

Requesting approval to revert the previous patch and check in this one.  This patch is the correct fix for the problem.  It's a polish fix for the Applications prefpane, and it shouldn't cause problems for other richlistboxes, as they too are vertically oriented, and none of them use listheaders.
Comment 24 User image Dão Gottwald [:dao] 2007-09-27 16:07:38 PDT
This bug has been marked as blocking.
Comment 25 User image Myk Melez [:myk] [@mykmelez] 2007-09-27 16:22:02 PDT
Comment on attachment 281843 [details] [diff] [review]
Proposed patch

(In reply to comment #24)
> This bug has been marked as blocking.

Ah, indeed.  In that case canceling approval request.  I'll check this in.
Comment 26 User image Myk Melez [:myk] [@mykmelez] 2007-09-27 16:27:05 PDT
I have reverted the original patch:

Checking in toolkit/content/widgets/richlistbox.xml;
/cvsroot/mozilla/toolkit/content/widgets/richlistbox.xml,v  <--  richlistbox.xml
new revision: 1.45; previous revision: 1.44
done

I haven't yet checked in the new patch, however, because of the following message at the top of toolkit/content/xul.css:

   THIS FILE IS LOCKED DOWN.  YOU ARE NOT ALLOWED TO MODIFY IT WITHOUT FIRST 
   HAVING YOUR CHANGES REVIEWED BY mconnor@steelgryphon.com
Comment 27 User image Steve England [:stevee] 2007-10-08 09:28:33 PDT
*** Bug 399012 has been marked as a duplicate of this bug. ***
Comment 28 User image Reed Loden [:reed] (use needinfo?) 2007-10-11 15:40:28 PDT
Comment on attachment 281843 [details] [diff] [review]
Proposed patch

mconnor gave this r+ over IRC. I'll land it now.
Comment 29 User image Reed Loden [:reed] (use needinfo?) 2007-10-11 15:43:44 PDT
Checking in toolkit/content/xul.css;
/cvsroot/mozilla/toolkit/content/xul.css,v  <--  xul.css
new revision: 1.104; previous revision: 1.103
done
Comment 30 User image Elmar Ludwig 2007-10-14 02:04:37 PDT
Verified fixed in Minefield/3.0a9pre ID:2007101214

Note You need to log in before you can comment on or make changes to this bug.