Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2560453002:
Prioritize hyphenations over break-word/break-all (Closed)
|
Created:
4 years ago by kojii Modified:
4 years ago Reviewers:
eae CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1 Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionPrioritize hyphenations over break-word/break-all
When these mid-word break properties are used together, this patch
tries to hyphenate first.
This behavior matches to WebKit, and to Gecko/Edge for break-word. The
priority isn't defined in the spec, currently an open issue[1]. But for
break-word, 4 browsers are interoperable with this change.
Also fixes one of failing cases in fast/text/basic/015.html that is
related with additionalWidthFromAncestors when break-word.
[1] https://github.com/w3c/csswg-drafts/issues/791
BUG=671341
Committed: https://crrev.com/0922f146b8701a3609d57e0fb11b3eb9d35ceb47
Cr-Commit-Position: refs/heads/master@{#436979}
Patch Set 1 #Patch Set 2 : Fix #
Total comments: 1
Patch Set 3 : Rebase with a static helper function #
Messages
Total messages: 28 (22 generated)
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== WIP: midword-priority BUG= ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch changes to try to hyphenate first. This change matches Blink behavior to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, all browsers are interoperable with this change. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ==========
kojii@chromium.org changed reviewers: + eae@chromium.org
PTAL. This one is rather rare and I'm less confident on the fix that I will not merge to M56.
Description was changed from ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch changes to try to hyphenate first. This change matches Blink behavior to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, all browsers are interoperable with this change. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ==========
LGTM, agree about not merging. Thank you for looking into this so promptly as always! Really appreciate it. https://codereview.chromium.org/2560453002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h (right): https://codereview.chromium.org/2560453002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/line/BreakingContextInlineHeaders.h:1104: if (hyphenation && (m_nextObject || isLineEmpty || This is pretty hard to read, how about adding a static helper function?
Description was changed from ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ==========
Description was changed from ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors when break-all/break-word. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ==========
Description was changed from ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors when break-all/break-word. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors when break-word. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ==========
The CQ bit was checked by kojii@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/07 at 07:16:20, eae wrote: > This is pretty hard to read, how about adding a static helper function? Done, in both this CL and the other one, thank you for your review to the point as always!
The CQ bit was checked by kojii@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org Link to the patchset: https://codereview.chromium.org/2560453002/#ps40001 (title: "Rebase with a static helper function")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481128476651780,
"parent_rev": "588fd0df3a96860d1dc847fefef03e950d9dc185", "commit_rev":
"b95d88892b040950d89c4c89795b840269c5fb64"}
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors when break-word. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 ========== to ========== Prioritize hyphenations over break-word/break-all When these mid-word break properties are used together, this patch tries to hyphenate first. This behavior matches to WebKit, and to Gecko/Edge for break-word. The priority isn't defined in the spec, currently an open issue[1]. But for break-word, 4 browsers are interoperable with this change. Also fixes one of failing cases in fast/text/basic/015.html that is related with additionalWidthFromAncestors when break-word. [1] https://github.com/w3c/csswg-drafts/issues/791 BUG=671341 Committed: https://crrev.com/0922f146b8701a3609d57e0fb11b3eb9d35ceb47 Cr-Commit-Position: refs/heads/master@{#436979} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0922f146b8701a3609d57e0fb11b3eb9d35ceb47 Cr-Commit-Position: refs/heads/master@{#436979} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
