Closed Bug 526478 Opened 10 years ago Closed 10 years ago

message header uses too many boxes

Categories

(Thunderbird :: Message Reader UI, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: alfredkayser, Unassigned)

References

(Blocks 1 open bug)

Details

Currently the message header consists of:
<hbox id="msgHeaderView">
  <deck id="msgHeaderViewDeck">
    <vbox id="expandedHeaderView">
      <vbox id="expandedHeadersBox>
        <hbox id="expandedHeadersTopBox>
           ...
        <hbox id="expandedHeadersBottomBox>
           ...
This could easily be replaced with:
<vbox id="msgHeaderView">
  <hbox id="msgHeaderTopBox>
     ...
  <hbox id="msgHeaderBottomBox>
     ...
removing three levels of unneeded boxes
(unless these message header fields really are trying to escape...)

The messageHeader display can still use every bit of optimization has it is still slow (in my humble perception...)


Same goes for the grid in the TopBox, which is also unneeded.
replace 
<hbox id="msgHeaderTopBox>
   <grid>
      ...
      <rows>
         <row>
            <label ...>
            <mail-multi-label...>
   <toolbox ...>
with just:
<hbox id="msgHeaderTopBox>
  <label ...>
  <mail-multi-label...>
  <toolbox ...>
Blocks: 466025
(In reply to comment #0)
> Currently the message header consists of:
> <hbox id="msgHeaderView">
>   <deck id="msgHeaderViewDeck">
>     <vbox id="expandedHeaderView">
>       <vbox id="expandedHeadersBox>
>         <hbox id="expandedHeadersTopBox>
>            ...
>         <hbox id="expandedHeadersBottomBox>
>            ...
> This could easily be replaced with:
> <vbox id="msgHeaderView">
>   <hbox id="msgHeaderTopBox>
>      ...
>   <hbox id="msgHeaderBottomBox>

No it can't. You're removing the deck which (afaik) is purposely included so that extensions can provide alternatives to the built-in message header view.

You might if you're lucky get rid of expandedHeaderView but that's about it.

> Same goes for the grid in the TopBox, which is also unneeded.

Nope. It is purposely done in the same manner as the bottom box so that we can align the widths of the first columns of the grids in the top and bottom boxes - so that the labels & data lines up.
The reason that expandedHeaderView and expandedHeadersBox are separate is described in bug 516033.

Marking as WONTFIX, as I don't think we currently have reason to believe that the number of boxes present are causing performance problems here.

That said, I think it would be tremendously valuable to use a profiler (perhaps the JS dtrace hooks and/or pecobro?) to understand exactly what's going on in any pieces that seem slow and where the big problems & low-hanging fruit are.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.