Closed Bug 780258 Opened 8 years ago Closed 8 years ago

Add support for max line box width API

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jwir3, Assigned: jwir3)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
Priority: -- → P1
Attached patch b780258 (WIP) (obsolete) — Splinter Review
wip patch
Attached file b780258 (WIP) (obsolete) —
Attachment #649449 - Attachment is obsolete: true
Depends on: 784887
Attached patch b780258Splinter Review
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.
Attachment #652767 - Attachment is obsolete: true
Attachment #656169 - Flags: review?(dbaron)
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
Attachment #656169 - Flags: review?(dbaron) → review+
Fixed requested changes and pushed to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a101c20b729d
Target Milestone: --- → mozilla18
https://hg.mozilla.org/mozilla-central/rev/a101c20b729d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Blocks: 813271
No longer blocks: 813271
Blocks: 818105
You need to log in before you can comment on or make changes to this bug.