Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to the handling of "position" and "positionAlign". #92

Closed
wants to merge 2 commits into from

Conversation

silviapfeiffer
Copy link
Member

Introduce both an "auto" value for "position" and "positionAlign"
settings which depends on the "text alignment".

Introduce a "computed text position" and "computed text position
alignment" algorithm such that these can be set by script through VTTCue.

This results in some simplifications in the rendering algorithm, in
particular we now don't need positionSet and positionAlignSet variables
any longer.

@silviapfeiffer
Copy link
Member Author

This is a proposed alternative to #69 . I have taken a lot of your proposed text from there, but based it on the current spec and also applied it to positionAlign, since I maintain that it is an important and necessary addition to allow the different setting dimensions to be dealt with independently when authoring.

I believe similar changes should be done for line and lineAlign.

I also believe that we can re-introduce the automatic size changes, but I would like to see that done through introduction of an "auto" keyword for "size", see https://www.w3.org/Bugs/Public/show_bug.cgi?id=25660 .

@@ -4456,34 +4440,30 @@
follows:</p>

<dl class="switch">
<!-- Q(Silvia): Why is "computed text position" dealt with again here??? The boxes are already
text positioned in step 8 and this should only be about percentage positioning the line position
direction. -->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be curious about feedback here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT this was the original issue in https://www.w3.org/Bugs/Public/show_bug.cgi?id=19178, namely that for snap-to-lines, all the previous positioning is ignored, instead positioning the bounding box of the text in both axes in a manner similar to CSS background-position. I think that bug should be reopened.

@foolip
Copy link
Member

foolip commented Jun 8, 2014

I would like this to be split into separate commits and branches, at least to this granularity:

  • Introducing position "auto"
  • Reverting positionSet and positionAlignSet (separate commit but maybe same branch as above)
  • Introducing positionAlign "auto"
  • "Apply position and positionAlign fixes also to regions"
  • Merging 'rtl' and 'ltr' for non-snap-to-lines
  • Typos and other drive-by fixes separately

Not necessarily in that order. Smaller commits makes reviewing easier and the resulting history easier to follow.

@silviapfeiffer
Copy link
Member Author

OK, fair enough. I'll see if I can get these done today.

Introduce both an "auto" value for "position" and "positionAlign"
settings which depends on the "text alignment".

Introduce a "computed text position" and "computed text position
alignment" algorithm such that these can be set by script through VTTCue.

This results in some simplifications in the rendering algorithm, in
particular we now don't need positionSet and positionAlignSet variables
any longer.
@silviapfeiffer
Copy link
Member Author

I'm gonna close this pull request, since we've pull out everything here into other pull requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants