780258 - Add support for max line box width API

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Scott Johnson (:jwir3)]

For Readability 2.0, we want to be able to specify a max width for line boxes throughout the page, then force a reflow of the entire page and wrap line boxes that are larger than this width, when this is changed. It should be able to be changed from within chrome javascript, but not content javascript.

Comment 1

[Scott Johnson (:jwir3)]

Attachment: b780258 (WIP)

wip patch

Comment 2

[Scott Johnson (:jwir3)]

Attachment: b780258 (WIP)

Comment 3

[Scott Johnson (:jwir3)]

Attachment: b780258

This version doesn't yet have a notification after the line box width has been changed - I'm not sure we're going to need that. I'll add it, though, if you think it's necessary before this patch lands.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 656169 [details] [diff] [review]
b780258

Don't overindent the contents of |struct LineBoxInfo|.

In nsIPresShell::SetMaxLineBoxWidth, I think you can use eResize
rather than eStyleChange, since intrinsic widths don't change as
a result of the max line box width.

You should probably just call the pres shell getter MaxLineBoxWidth
and not use the word "Get", since I think our preference is to reserve
the word Get for things that are fallible (might return null).

In layout/base/tests/chrome/Makefile.in, you should match the
indentation level and character used (probably tabs) of the
surroundings.

But I actually think you don't want to write a chrome mochitest;
you should write a regular mochitest, add a getter/setter for
maxLineBoxWidth to SpecialPowers (like there is for textZoom
and fullZoom).  Or am I missing some reason you need a chrome
mochitest here?

Also, in the test, do you:
 * need to unset the width that you set?
 * need to set to display:none a second time at some point?  (Though
   I'm not sure why you're messing with display at all... I'd think
   you shouldn't need to.)

In nsLineLayout::ReflowFrame, you should get the pres context out
of mPresContext rather than aFrame->PresContext().

r=dbaron with that fixed

Comment 5

[Scott Johnson (:jwir3)]

Fixed requested changes and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a101c20b729d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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