836969 - Kill ContentAreaObserver

Status: RESOLVED WONTFIX

(Firefox for Metro Graveyard :: Theme, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

We currently size some of our UI elements dynamically using a complex bit of JavaScript called ContentAreaObserver.  I think we can get rid of this and just have Gecko do our layout.  It would let us remove a big chunk of code that has already been the cause of a number of fiddly little bugs.

This WIP patch is almost fully-functional.  TODO: Fix the browser <notificationbox> functionality, and deal with the on-screen keyboard.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: WIP 2

Fixed the <notificationbox>.

Comment 2

[Matt Brubeck (:mbrubeck)]

I split this up into several smaller patches.  Some of the patches are fairly simple and fix known UI problems, so I filed separate bugs for them and requested review.  I'll put the remaining patches here for posterity, but I'm not planning to land them.  Instead I'll summarize what I learned:

The reason ContentAreaObserver exists is because XUL Fennec originally had a main UI that was larger than the screen (because of the sliding off-screen sidebars and toolbar), but some elements within that UI needed to be sized to the screen.  We couldn't use normal layout to achieve this, so we added some JavaScript to explicitly set the dimensions of those elements to match the screen.  It then grew some extra features like tracking toolbar and on-screen keyboard heights.

Metro Firefox does not face the same problem; our UI is sized exactly to the screen, so we could use the good old flexible box model to lay it out.  I wanted to do so because the current code requires us to keep ContentAreaObserver in sync with the actual window size and UI state at all times; any failure in this discipline leads to layout bugs like bug 836458.  And our current layout is kind of weird and fragile, which leads to bugs like bug 816552 and bug 816316.  I hoped that using a typical flex-box layout would simplify this and eliminate some of those possibilities.

However, there's one wrinkle: Because we have no widget-level windows/popups, we implement popups/dialogs/flyouts/overlays by putting everything into a big XUL <stack>.

The <stack> element has its own sizing model, which causes changes in one child's dimension or positions to propagate up to the <stack> and then down to other children.  This makes it very easy to introduce layout bugs in part of the UI by changing a completely unrelated part.  Keeping some explicit dimensions can actually protect us from *some* (though not all) of these stack-sizing bugs.  Without ContentAreaObserver I expect we'd spend even more time hunting down these bugs and adding -moz-stack-sizing styles to work around them.

I was not able to significantly reduce our dependence on the <stack>, though I tried quite hard!  I hoped that we could replace some stack usage with absolute positioning, but this plays really badly with stack layout and flex box layout for reasons that don't seem to be well-documented, so I gave up.

Finally, we still need to keep some of features of ContentAreaObserver, like resizing elements when the keyboard appears, and dispatching events after certain layout changes.

In summary, this bug would simplify our code, but not as much as I hoped. It would eliminate one class of bugs (out of sync state in JS), but it might worsen another class of bugs (layout issues due to stack sizing).  Overall, probably not worth the churn.  Instead, I'll focus on making sure any new UI (especially the new main UI in bug 835622) is built as simply as possible to avoid too much dependence on either stack sizing or JS state.

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: 1: flex ALL THE THINGS

Remove explicit height/width styles, replacing them with flex as needed.

[These patches depend on the patches in bug 838222, bug 837396, and bug 837436.]

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: 2: finishing blow

Now that we don't depend on the styles managed by ContentAreaObserver, we can rid ourselves of it for good.

Comment 5

[Matt Brubeck (:mbrubeck)]

Attachment: 3: resurrection

We do still want to keep content above the keyboard.  This patch restores that feature, using a <spacer> element to represent the keyboard within our layout.  (This is one of the places where stack sizing became a real pain.)