Open Bug 1310106 Opened 8 years ago Updated 1 year ago

Implement the rendering of floating :first-letter with CSS initial-letter property

Categories

(Core :: Layout: Text and Fonts, defect, P3)

Product:
Core
Component:
Layout: Text and Fonts
Type:
defect

Tracking

()

People

(Reporter: chenpighead, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(2 files)

Bug 1310106 - sink lines of texts to achieve a raised/sunken initial.
58 bytes, text/x-review-board-request
dbaron
: review-
Details
Bug 1310106 - sink initial letter to achieve a dropped initial (or drop cap).
58 bytes, text/x-review-board-request
dbaron
: review-
Details 
At present, we haven't decided if we should force :first-letter with CSS initial-letter property to be float. So, we need to implement the rendering of both floating and non-floating :first-letter with initial-letter property.

In this bug, I'll try to render floating :first-letter with initial-letter property. As to the non-floating case, we may or may not need another bug. It depends on the study and the implementation in this bug.
Summary: Implement the rendering of floating CSS initial-letter → Implement the rendering of floating :first-letter with CSS initial-letter property
> At present, we haven't decided if we should force :first-letter with CSS initial-letter property to be float.

Fwiw, forcing a first-letter box to be a float seems undesirable to me.
first-letter floats are problematic since they affect placement of other
floats on the page which is often undesirable.
(In reply to Mats Palmgren (:mats) from comment #1)
> > At present, we haven't decided if we should force :first-letter with CSS initial-letter property to be float.
> 
> Fwiw, forcing a first-letter box to be a float seems undesirable to me.
> first-letter floats are problematic since they affect placement of other
> floats on the page which is often undesirable.

Mats, thank you for the comment. I believe we are on the same page.

As discussed with David in London All hands, due to the same reason as you mentioned, forcing a first-letter to be a float is undesirable to him as well.

Accordingly, I would try to implement the rendering of CSS initial-letter property without forcing a first-letter to be float. Which means we need to separately do both cases (floating first-letter and non-floating first-letter). This bug should only cover the case that a :first-letter with both float and initial-letter set.

I guess comment 0 could've been more detailed by adding a sentence like "Although forcing :first-letter with CSS initial-letter property to be float could be an implementation option, we'd prefer not to if possible."
Comment on attachment 8806340 [details]
Bug 1310106 - sink lines of texts to achieve a raised/sunken initial.

For floating :first-letter with CSS initial-letter property case, the floating initial letter is placed at the top-left (assuming float:left) corner of its block container. In order to implement a Sunken/Raised initial letter, I think we should move the lines to align with the baseline of the initial letter instead of moving the floating initial letter.

In this patch, I'd like to apply initial-letter property's sink argument at the beginning of the LineLayout's reflow process. There're two problems remained:

1. I'm not sure if we should consider a case that the sink argument is larger than the size argument, which represents a drop much lower initial. If so, I might need another patch to deal with it.
2. An assertion [1] is hit. I might need to dig deeper to see if I break something.

Hi Jonathan, I'm not sure if I'm going toward the right direction. Could you take a look and give me some feedback? Thanks.


[1] http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/layout/generic/nsBlockFrame.cpp#4467
Attachment #8806340 - Flags: feedback?(jfkthame)
(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #4)
> Comment on attachment 8806340 [details]
> Bug 1310106 - wip
> 
> For floating :first-letter with CSS initial-letter property case, the
> floating initial letter is placed at the top-left (assuming float:left)
> corner of its block container. In order to implement a Sunken/Raised initial
> letter, I think we should move the lines to align with the baseline of the
> initial letter instead of moving the floating initial letter.

This looks promising, but you may need to constrain things such that the following lines are never actually _raised_ to align to the initial letter, otherwise this can result in unexpectedly _reducing_ the line spacing between paragraphs where the raised initial occurs. This will happen if the line-height is large enough that the float is not as big as the ascent of the line:

data:text/html,<style>p{font:16px/2.5 Arial; width:10em; margin:0;} p:first-letter{initial-letter:1.1; float:left;}</style><p>The quick brown fox jumps backwards over the lazy dog.<p>The quick brown fox jumps backwards over the lazy dog.

I don't think we ever want the presence of an initial letter to have the effect of suppressing the line-height that would otherwise be used. So in a case like this where the existing ascent of the line is larger than the ascent of the floated frame (which wraps the initial letter -without- extra inter-line spacing, AFAICS) you'd need to push the float downwards (probably by increasing its ascent).

Then, I suspect this becomes quite similar to the case where sink is larger than size:

> 1. I'm not sure if we should consider a case that the sink argument is larger than the size argument,
> which represents a drop much lower initial. If so, I might need another patch to deal with it.

There, too, the height of the float needs to be increased so that it sinks to the right baseline without the following text needing to be raised. Otherwise, given an example like:

data:text/html,<style>p:first-letter{initial-letter:2 3; float:left;}</style><p>The quick brown fox jumps backwards over the lazy dog.

if the float is shrink-wrapped and kept at the top left of its container, then the following text will be raised way above the container. I don't think that's right; in this example, the continuation should stay at its normal baseline and the floated frame should have its ascent increased so as to "push" the letter down to the 3rd baseline.
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> This looks promising, but you may need to constrain things such that the
> following lines are never actually _raised_ to align to the initial letter,
> otherwise this can result in unexpectedly _reducing_ the line spacing
> between paragraphs where the raised initial occurs. This will happen if the
> line-height is large enough that the float is not as big as the ascent of
> the line:
> 
> data:text/html,<style>p{font:16px/2.5 Arial; width:10em; margin:0;}
> p:first-letter{initial-letter:1.1; float:left;}</style><p>The quick brown
> fox jumps backwards over the lazy dog.<p>The quick brown fox jumps backwards
> over the lazy dog.
> 
> I don't think we ever want the presence of an initial letter to have the
> effect of suppressing the line-height that would otherwise be used. So in a
> case like this where the existing ascent of the line is larger than the
> ascent of the floated frame (which wraps the initial letter -without- extra
> inter-line spacing, AFAICS) you'd need to push the float downwards (probably
> by increasing its ascent).

Yes, you're right. I plan to do this adjustment while reflowing the initial letter (somewhere in nsFirstLetterFrame.cpp).

> Then, I suspect this becomes quite similar to the case where sink is larger
> than size:
> 
> > 1. I'm not sure if we should consider a case that the sink argument is larger than the size argument,
> > which represents a drop much lower initial. If so, I might need another patch to deal with it.
> 
> There, too, the height of the float needs to be increased so that it sinks
> to the right baseline without the following text needing to be raised.
> Otherwise, given an example like:
> 
> data:text/html,<style>p:first-letter{initial-letter:2 3;
> float:left;}</style><p>The quick brown fox jumps backwards over the lazy dog.
> 
> if the float is shrink-wrapped and kept at the top left of its container,
> then the following text will be raised way above the container. I don't
> think that's right; in this example, the continuation should stay at its
> normal baseline and the floated frame should have its ascent increased so as
> to "push" the letter down to the 3rd baseline.

Hm... I wonder how to get this kind of information (the sink value is larger than the actual line number) while reflowing the initial letter... Do you have any idea to do this?
Flags: needinfo?(jfkthame)
(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #6)
> (In reply to Jonathan Kew (:jfkthame) from comment #5)
> > This looks promising, but you may need to constrain things such that the
> > following lines are never actually _raised_ to align to the initial letter,
> > otherwise this can result in unexpectedly _reducing_ the line spacing
> > between paragraphs where the raised initial occurs. This will happen if the
> > line-height is large enough that the float is not as big as the ascent of
> > the line:
> > 
> > data:text/html,<style>p{font:16px/2.5 Arial; width:10em; margin:0;}
> > p:first-letter{initial-letter:1.1; float:left;}</style><p>The quick brown
> > fox jumps backwards over the lazy dog.<p>The quick brown fox jumps backwards
> > over the lazy dog.
> > 
> > I don't think we ever want the presence of an initial letter to have the
> > effect of suppressing the line-height that would otherwise be used. So in a
> > case like this where the existing ascent of the line is larger than the
> > ascent of the floated frame (which wraps the initial letter -without- extra
> > inter-line spacing, AFAICS) you'd need to push the float downwards (probably
> > by increasing its ascent).
> 
> Yes, you're right. I plan to do this adjustment while reflowing the initial
> letter (somewhere in nsFirstLetterFrame.cpp).
> 
> > Then, I suspect this becomes quite similar to the case where sink is larger
> > than size:
> > 
> > > 1. I'm not sure if we should consider a case that the sink argument is larger than the size argument,
> > > which represents a drop much lower initial. If so, I might need another patch to deal with it.
> > 
> > There, too, the height of the float needs to be increased so that it sinks
> > to the right baseline without the following text needing to be raised.
> > Otherwise, given an example like:
> > 
> > data:text/html,<style>p:first-letter{initial-letter:2 3;
> > float:left;}</style><p>The quick brown fox jumps backwards over the lazy dog.
> > 
> > if the float is shrink-wrapped and kept at the top left of its container,
> > then the following text will be raised way above the container. I don't
> > think that's right; in this example, the continuation should stay at its
> > normal baseline and the floated frame should have its ascent increased so as
> > to "push" the letter down to the 3rd baseline.
> 
> Hm... I wonder how to get this kind of information (the sink value is larger
> than the actual line number) while reflowing the initial letter... Do you
> have any idea to do this?

I just thought through this, and both cases should be well taken care of in one same place.
Just like you said, these two cases are quite similar.
Thank you for the feedback. :-)
Flags: needinfo?(jfkthame)
Just discussed w/ David offline, and the possibility of ignoring float property when float and initial-letter are set on the same element is filed (please see https://github.com/w3c/csswg-drafts/issues/688). 

This might change the implementation plan so far, so I'd be better see how the discussion goes before going any further.
So it still might make sense to use some of the code you've written here, and then try to make further changes (like putting the initial letter in its own line box) later in order to support initial-letter on <span></span> in addition to supporting it on ::first-letter.

The biggest point I want to emphasize is that the answer to the second paragraph of comment 0 is that we should *not* need another bug.  If you can render initial-letter (which is essentially CSS's third type of first-letter effect) correctly using something based on one of the two sets of first-letter code today (non-floating or floating), that's probably OK to start from.

However, I would probably have picked the non-floating code to start from rather than the floating code, since you need to place the initial letter according to the initial letter rules rather than according to the rules for float placement, and to make the first letter an exclusion for line boxes without having it affect the placement of other floats.
(I think the long term plan, if we want to support initial-letter on inline elements in addition to ::first-letter (do other browsers?), is that we'd need to put the initial letter in its own line box.  But that might not be necessary for just doing the support for ::first-letter, and it might be a lot of extra complexity, so it may be worth doing only ::first-letter support without doing that.)
(In reply to David Baron :dbaron: ⌚️UTC+8 from comment #14)
> (I think the long term plan, if we want to support initial-letter on inline
> elements in addition to ::first-letter (do other browsers?), is that we'd
> need to put the initial letter in its own line box.  But that might not be
> necessary for just doing the support for ::first-letter, and it might be a
> lot of extra complexity, so it may be worth doing only ::first-letter
> support without doing that.)

No, the only browser that supports initial-letter is Safari, and it only supports ::first-letter for now. So, supporting ::first-letter only is the goal at present.

As you said in comment 13, I can render initial-letter correctly using something based on one of the two sets of first-letter code today (non-floating or floating). The reason why I choose the non-floating is that a floating ::first-letter's baseline position would not affect its continuation lines' position. I guess that's one of the reasons why Safari implement it by forcing ::first-letter to be float internally.

I thought I need to put the initial letter in its own line box to render initial-letter with the non-floating code. But, maybe it is not necessary? I might need some tests to verify if I can make it w/o putting the initial letter in its own line box.
(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #15)
> 
> No, the only browser that supports initial-letter is Safari, and it only
> supports ::first-letter for now. So, supporting ::first-letter only is the
> goal at present.
> 
> As you said in comment 13, I can render initial-letter correctly using
> something based on one of the two sets of first-letter code today
> (non-floating or floating). The reason why I choose the non-floating is that

  s/choose/choose to starts from

> a floating ::first-letter's baseline position would not affect its
> continuation lines' position. I guess that's one of the reasons why Safari
> implement it by forcing ::first-letter to be float internally.
> 
> I thought I need to put the initial letter in its own line box to render
> initial-letter with the non-floating code. But, maybe it is not necessary? I
> might need some tests to verify if I can make it w/o putting the initial
> letter in its own line box.
(In reply to David Baron :dbaron: ⌚️UTC+8 from comment #13)
> and to make the first letter an exclusion for line boxes
> without having it affect the placement of other floats.

Doesn't the behavior described under "Interaction with floats" in css-inline[1] imply that initial letters _do_ affect placement of other floats, given the requirement for floats to clear the initial? The draft marks this as an open issue, however, so perhaps that's not where we'll end up.

[1] https://drafts.csswg.org/css-inline/#initial-letter-floats
Comment on attachment 8806340 [details]
Bug 1310106 - sink lines of texts to achieve a raised/sunken initial.

David, would it make more sense for you to discuss/review this with Jeremy?
Attachment #8806340 - Flags: review?(jfkthame) → review?(dbaron)
Attachment #8807433 - Flags: review?(jfkthame) → review?(dbaron)
(But if you just want to bounce it back to me, I'll take another look...)
Comment on attachment 8806340 [details]
Bug 1310106 - sink lines of texts to achieve a raised/sunken initial.

https://reviewboard.mozilla.org/r/89812/#review93258

So as I said earlier, I think we should be using the non-floating first letter code as the basis for initial-letter support, not the floating first letter code, since using the floating first-letter code will make it hard to get interactions with floats correct.  (Adding additional exclusions to the float manager (or elsewhere) for the space occupied by the initial letter should be somewhat more straightforward.)

This patch is also wrong in a second substantial way.  Moving mBStart down is the wrong approach, since that assumes that nothing is present on the first line that extends above its line height.  This isn't necessarily true, e.g. if you have:
  <p>This line has <span>larger text</span>.</p>
or:
  <p>This line <img> has a tall image.</p>
with the p having initial-letter styles.

Instead, sunken initial letters, raised initial letters, and the handling of the margin-box of drop caps, all as described in https://drafts.csswg.org/css-inline/#raised-sunken-caps, should be handled by increasing the space at the top of the line *if needed* in nsLineLayout::VerticalAlignLine.
Attachment #8806340 - Flags: review?(dbaron) → review-
One further clarification on all my comments here:  20% of them are probably wrong, since I haven't actually implemented this.  But I think they're the direction you should start out with, and then discover what's wrong with them.
(In reply to Jonathan Kew (:jfkthame) from comment #17)
> (In reply to David Baron :dbaron: ⌚️UTC+8 from comment #13)
> > and to make the first letter an exclusion for line boxes
> > without having it affect the placement of other floats.
> 
> Doesn't the behavior described under "Interaction with floats" in
> css-inline[1] imply that initial letters _do_ affect placement of other
> floats, given the requirement for floats to clear the initial? The draft
> marks this as an open issue, however, so perhaps that's not where we'll end
> up.
> 
> [1] https://drafts.csswg.org/css-inline/#initial-letter-floats

So that's certainly true for floats whose placeholder is on a line after the first line but that are affected by an initial letter.  It's less clear, as the issue notes, whether this should also happen if the placeholder/anchor of the float is in the first line.

However, the behavior specified is *clearing* the initial letter, not interacting with it as though it's a float (which would allow placing next to it).

This behavior is necessary because the initial letter means that the text can't wrap around floats as normal, because the initial letter begins at a single position, which is the same for all the lines it is adjacent to.
Comment on attachment 8807433 [details]
Bug 1310106 - sink initial letter to achieve a dropped initial (or drop cap).

https://reviewboard.mozilla.org/r/90576/#review93268

In addition to being based on the floating first-letter code (which I think is wrong, as stated above), I think the approach this patch takes is fundamentally incorrect because it's trying to keep the position of the first letter frame the same, and move the child within the first letter frame.  That's wrong both because it's not the model the spec has (which means you're likely to run into a series of issues), and because it obviously produces the wrong results for cases like the ::first-letter pseudo-element having a background specified.

While maybe some low-level bits of this patch might turn out useful later, I think the basic approach taken here should be discarded.

::: layout/generic/nsFirstLetterFrame.cpp:234
(Diff revision 2)
> +      nsStyleContext* containerSC = mStyleContext->GetParent();
> +      const nsStyleDisplay* containerDisp = containerSC->StyleDisplay();
> +      while (containerDisp->mDisplay == mozilla::StyleDisplay::Contents) {
> +        if (!containerSC->GetParent()) {
> +          break;
> +        }
> +        containerSC = containerSC->GetParent();
> +        containerDisp = containerSC->StyleDisplay();
> +      }

This is unnecessary; you should find the containing frame from the frame tree or from the containing block stored in the reflow state, and get its style.
Attachment #8807433 - Flags: review?(dbaron) → review-
(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #20)
> Comment on attachment 8806340 [details]
> Bug 1310106 - sink lines of texts to achieve a raised/sunken initial.
> 
> https://reviewboard.mozilla.org/r/89812/#review93258
> 
> So as I said earlier, I think we should be using the non-floating first
> letter code as the basis for initial-letter support, not the floating first
> letter code, since using the floating first-letter code will make it hard to
> get interactions with floats correct.  (Adding additional exclusions to the
> float manager (or elsewhere) for the space occupied by the initial letter
> should be somewhat more straightforward.)
> 
> This patch is also wrong in a second substantial way.  Moving mBStart down
> is the wrong approach, since that assumes that nothing is present on the
> first line that extends above its line height.  This isn't necessarily true,
> e.g. if you have:
>   <p>This line has <span>larger text</span>.</p>
> or:
>   <p>This line <img> has a tall image.</p>
> with the p having initial-letter styles.
> 
> Instead, sunken initial letters, raised initial letters, and the handling of
> the margin-box of drop caps, all as described in
> https://drafts.csswg.org/css-inline/#raised-sunken-caps, should be handled
> by increasing the space at the top of the line *if needed* in
> nsLineLayout::VerticalAlignLine.

This just reminds me that the only implemented browser, Safari, also has this issue. Filed https://github.com/w3c/csswg-drafts/issues/735.
Unassigning myself from the bug as I don't quite have time working on this at the moment... Anyone interested can pick it up.
Assignee: jeremychen → nobody
Status: ASSIGNED → NEW
Priority: -- → P3
Adding ddn, just in case.
Keywords: dev-doc-needed
Severity: normal → S3

Chromium recently decided to ship the CSS initial-letter property:

https://groups.google.com/a/chromium.org/g/blink-dev/c/1chZ62qRzIY/m/KIWHYZ_kBQAJ

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: