Add support for max line box width API

RESOLVED FIXED in mozilla18

Status

()

Core
Layout
P1
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jwir3, Assigned: jwir3)

Tracking

(Blocks: 1 bug)

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Updated

6 years ago
Priority: -- → P1
(Assignee)

Comment 1

6 years ago
Created attachment 649449 [details] [diff] [review]
b780258 (WIP)

wip patch
(Assignee)

Comment 2

6 years ago
Created attachment 652767 [details]
b780258 (WIP)
Attachment #649449 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Depends on: 784887
(Assignee)

Comment 3

6 years ago
Created attachment 656169 [details] [diff] [review]
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.
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+
(Assignee)

Comment 5

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Blocks: 813271
(Assignee)

Updated

6 years ago
No longer blocks: 813271
(Assignee)

Updated

6 years ago
Blocks: 818105
You need to log in before you can comment on or make changes to this bug.