502367 - Error: tree.columns is undefined,when click column header of applications pane

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090703 Firefox/3.5.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090703 Firefox/3.5.1pre ID:20090703042842

An error "Error: tree.columns is undefined" shown in error console,when click column header of apprications pane.


Reproducible: Always

Steps to Reproduce:
1. Start Shiretoko 3.5.1pre with new profile.
2. set "javascript.options.strict" to true
          "javascript.options.showInConsole"to true);
   (error to be shown in error console).
3. Tools > Error Console  (Show Error Console Window).
4. Tools > Options...     (Show Options Window).
5. Select Applicatios pane.
6. Click "Content Type" Header (change sorting order).

 

Actual Results:  
Error in Error console.

Warning: reference to undefined property tree.columns
Source File: chrome://global/content/bindings/tree.xml
Line: 1303
Error: tree.columns is undefined
Source File: chrome://global/content/bindings/tree.xml
Line: 1303


Expected Results:  
No error.

Same error shows on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090703 Minefield/3.6a1pre ID:20090703050112.

Comment 1

[u88484]

Confirmed.  Line 1329 using trunk build.

Comment 2

[Felix Fung (:felix)]

Attachment: Pref>Applications: Replace treecol With listheader Inside richlistbox

Summary of Changes:
- When sorting on the <treecol>, the element expected (reasonably) to be inside a treebox but instead got stuck inside a <treecol>. I just replaced the tree element with the appropriate <listheader> element.

Note: This problem was less evident in the original version but the icons in the action column overflow the width of the column. Hoping that reviewers can clue me into how I can get the headers adjusted to contain them.

Comment 3

[Felix Fung (:felix)]

Attachment: Screenshot showing action column being overflowd by icon

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 561357 [details] [diff] [review]
Pref>Applications: Replace treecol With listheader Inside richlistbox

Sorry for the delay reviewing the patch!

This fixes the reported problem but has some problematic side-effects.

First, it makes the column headers scroll with the rest of the rows when the list grows long enough to be scrolling.  If I remember correctly, the list is typically scrolling on Windows/Linux but too short to scroll on Mac. However, you might be able to see it on Mac by setting browser.download.show_plugins_in_list to true (if it isn't already).

Second, as you note, it exacerbates the issue that the column headers are misaligned with their columns.  The reason that happens is that richlistboxes don't actually support column headers, so we employ a hack (a single listheader containing two treecols of equal width) to obtain them.

And that hack is unfortunately imperfect.  In addition to the misalignment, it means that the column header widths change when you change the sort column, probably because of the movement of the sort direction indicator from one column to another.

The long term solution is to add support for column headers to richlistboxes.  Alternately, we could change the representation of information in this richlistbox to a single column layout (like the other richlistboxes in the product, f.e. in the Downloads dialog).

Unfortunately, I don't know of a good short-term workaround.  I suppose the Applications pane could set the header column widths programmatically based on whether or not there is a scrollbar (and update them when the Preference dialog window changes size or the number of shown rows changes).

Comment 5

[Tom Doan]

I think this can be fixed by modifying chrome://global/content/bindings/tree.xml (Line 1367 in Firefox v18)...

From:

          var tree = this.parentNode.parentNode;
          var column = tree.columns.getColumnFor(this);
          tree.view.cycleHeader(column);

To something like this:

          var tree = this.parentNode.parentNode;
          if (tree.columns != undefined) {
            var column = tree.columns.getColumnFor(this);
            tree.view.cycleHeader(column);
          }

Comment 6

[Mike Kaply [:mkaply]]

I'm using this same method for my add-on, and what I find even weirder is that even if I event.preventDefault and event.stopPropagation on the onclick, I still get the error.

I agree with Tom, though. We should check for tree.columns.

Comment 7

[Alice0775 White]

no preference window now.

Comment 8

[Mike Kaply [:mkaply]]

This happens with the new preference pane as well.

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Just add an if statement around tree columns

Simple patch to add a check for tree.columns before using it.

Comment 10

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/fx-team/rev/c90fe6f633f400db7eb108f02397bcc391c12a0d
Bug 502367 - tree.columns can be undefined so check for it; r=enndeakin

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c90fe6f633f4