130552 2014-03-20 18:02:33 -0700 Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes 2014-03-25 12:48:03 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED DUPLICATE 130540 P2 Normal --- 1 mmaxfield mmaxfield commit-queue dino esprehn+autocc glenn hyatt jonlee kondapallykalyan simon.fraser thorton oldest_to_newest 992841 0 mmaxfield 2014-03-20 18:02:33 -0700 Document assumptions made in TrailingObjects::updateMidpointsForTrailingBoxes 992842 1 227366 mmaxfield 2014-03-20 18:05:06 -0700 Created attachment 227366 Patch 992843 2 227366 simon.fraser 2014-03-20 18:09:56 -0700 Comment on attachment 227366 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227366&action=review > Source/WebCore/rendering/line/TrailingObjects.cpp:56 > + // There is something subtle going on here. This function gets called in two cases: > + // 1) In BreakingContext::handleEndOfLine(). The offset below can be 0 > + // in this case (meaning that the code below sets the offset to 4B), but that is okay > + // because BreakingContext::handleEndOfLine() immediately fixes up the offset by calling > + // increment() on the iterator, which detects offsets that are beyond the end of its node > + // and moves the iterator to the next node. > + // 2) When BreakingContext::handleText() detects multiple whitespace characters in a row. In > + // that case, the only way that the offset can be 0 is if the first two characters are > + // whitespace. However, LineBreaker::nextSegmentBreak() calls > + // LineBreaker::skipLeadingWhitespace() to skip over such whitespace before calling > + // BreakingContext::handleText(). > + // Therefore, this -1 wrapping is benign (albeit brittle). I think it would be better to make the code more self-documenting (and safe) than having to write a comment like this. > Source/WebCore/rendering/line/TrailingObjects.cpp:57 > lineMidpointState.midpoints()[trailingSpaceMidpoint].setOffset(lineMidpointState.midpoints()[trailingSpaceMidpoint].offset() -1); You should add a space before the 1 992848 3 227366 mmaxfield 2014-03-20 18:21:07 -0700 Comment on attachment 227366 Patch Rather than documenting bad design, I should fix the bad design. 994283 4 mmaxfield 2014-03-25 12:48:03 -0700 *** This bug has been marked as a duplicate of bug 130540 *** 227366 2014-03-20 18:05:06 -0700 2014-03-20 18:21:07 -0700 Patch bug-130552-20140320180444.patch text/plain 2966 mmaxfield U3VidmVyc2lvbiBSZXZpc2lvbjogMTY2MDA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOWU4NmY2MmYyMGMxMjFk MWM0YmU4ZjU3MDRkOTk5MDVlYzM2MTI4NC4uZTNlOTlmNGU0OTIyMzJjNzVjOGZjYmQxZTc1ZWIw ZDQyMWEwYjY3OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDE0LTAzLTIwICBNeWxl cyBDLiBNYXhmaWVsZCAgPG1tYXhmaWVsZEBhcHBsZS5jb20+CisKKyAgICAgICAgRG9jdW1lbnQg YXNzdW1wdGlvbnMgbWFkZSBpbiBUcmFpbGluZ09iamVjdHM6OnVwZGF0ZU1pZHBvaW50c0ZvclRy YWlsaW5nQm94ZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEzMDU1MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgIEFkZGluZyBhIGNvbW1lbnQgZGVzY3JpYmluZyB3aHkgZGVjcmVtZW50aW5nIGFuIElubGlu ZUl0ZXJhdG9yIHdoaWNoIGNvdWxkCisgICAgICAgIGhhdmUgYSAwIG9mZnNldCBpcyBiZW5pZ24u IEl0IGlzIGJldHRlciB0byBoYXZlIHRoZXNlIHN1YnRsZSBpbnRlcmFjdGlvbnMKKyAgICAgICAg d3JpdHRlbiBkb3duIHNvbWV3aGVyZS4KKworICAgICAgICBObyBuZXcgdGVzdHMgYmVjYXVzZSB0 aGVyZSBpcyBubyBiZWhhdmlvciBjaGFuZ2UuCisKKyAgICAgICAgKiByZW5kZXJpbmcvbGluZS9U cmFpbGluZ09iamVjdHMuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VHJhaWxpbmdPYmplY3RzOjp1 cGRhdGVNaWRwb2ludHNGb3JUcmFpbGluZ0JveGVzKToKKwogMjAxNC0wMy0yMCAgUHJhdGlrIFNv bGFua2kgIDxwc29sYW5raUBhcHBsZS5jb20+CiAKICAgICAgICAgaU9TIGJ1aWxkIGZpeCBhZnRl ciByMTY1OTkyLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL2xpbmUvVHJh aWxpbmdPYmplY3RzLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9saW5lL1RyYWlsaW5n T2JqZWN0cy5jcHAKaW5kZXggOTY0OTBjZDViZDFlZDdiYWVkMjcxNTU0Mzc2YjNmOTdhYzUxMTVh NC4uYzIxYmFkMjRhNDJhMDdkNzQ2ZDQ1YzBhMDFiYzhjZjJhMjEyYmMxMSAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL2xpbmUvVHJhaWxpbmdPYmplY3RzLmNwcAorKysgYi9T b3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvbGluZS9UcmFpbGluZ09iamVjdHMuY3BwCkBAIC00Miw2 ICs0MiwxOCBAQCB2b2lkIFRyYWlsaW5nT2JqZWN0czo6dXBkYXRlTWlkcG9pbnRzRm9yVHJhaWxp bmdCb3hlcyhMaW5lTWlkcG9pbnRTdGF0ZSYgbGluZU1pZAogICAgICAgICBmb3IgKCA7IHRyYWls aW5nU3BhY2VNaWRwb2ludCA+IDAgJiYgbGluZU1pZHBvaW50U3RhdGUubWlkcG9pbnRzKClbdHJh aWxpbmdTcGFjZU1pZHBvaW50XS5yZW5kZXJlcigpICE9IG1fd2hpdGVzcGFjZTsgLS10cmFpbGlu Z1NwYWNlTWlkcG9pbnQpIHsgfQogICAgICAgICBBU1NFUlQodHJhaWxpbmdTcGFjZU1pZHBvaW50 ID49IDApOwogICAgICAgICBpZiAoY29sbGFwc2VGaXJzdFNwYWNlID09IENvbGxhcHNlRmlyc3RT cGFjZSkKKyAgICAgICAgICAgIC8vIFRoZXJlIGlzIHNvbWV0aGluZyBzdWJ0bGUgZ29pbmcgb24g aGVyZS4gVGhpcyBmdW5jdGlvbiBnZXRzIGNhbGxlZCBpbiB0d28gY2FzZXM6CisgICAgICAgICAg ICAvLyAxKSBJbiBCcmVha2luZ0NvbnRleHQ6OmhhbmRsZUVuZE9mTGluZSgpLiBUaGUgb2Zmc2V0 IGJlbG93IGNhbiBiZSAwCisgICAgICAgICAgICAvLyBpbiB0aGlzIGNhc2UgKG1lYW5pbmcgdGhh dCB0aGUgY29kZSBiZWxvdyBzZXRzIHRoZSBvZmZzZXQgdG8gNEIpLCBidXQgdGhhdCBpcyBva2F5 CisgICAgICAgICAgICAvLyBiZWNhdXNlIEJyZWFraW5nQ29udGV4dDo6aGFuZGxlRW5kT2ZMaW5l KCkgaW1tZWRpYXRlbHkgZml4ZXMgdXAgdGhlIG9mZnNldCBieSBjYWxsaW5nCisgICAgICAgICAg ICAvLyBpbmNyZW1lbnQoKSBvbiB0aGUgaXRlcmF0b3IsIHdoaWNoIGRldGVjdHMgb2Zmc2V0cyB0 aGF0IGFyZSBiZXlvbmQgdGhlIGVuZCBvZiBpdHMgbm9kZQorICAgICAgICAgICAgLy8gYW5kIG1v dmVzIHRoZSBpdGVyYXRvciB0byB0aGUgbmV4dCBub2RlLgorICAgICAgICAgICAgLy8gMikgV2hl biBCcmVha2luZ0NvbnRleHQ6OmhhbmRsZVRleHQoKSBkZXRlY3RzIG11bHRpcGxlIHdoaXRlc3Bh Y2UgY2hhcmFjdGVycyBpbiBhIHJvdy4gSW4KKyAgICAgICAgICAgIC8vIHRoYXQgY2FzZSwgdGhl IG9ubHkgd2F5IHRoYXQgdGhlIG9mZnNldCBjYW4gYmUgMCBpcyBpZiB0aGUgZmlyc3QgdHdvIGNo YXJhY3RlcnMgYXJlCisgICAgICAgICAgICAvLyB3aGl0ZXNwYWNlLiBIb3dldmVyLCBMaW5lQnJl YWtlcjo6bmV4dFNlZ21lbnRCcmVhaygpIGNhbGxzCisgICAgICAgICAgICAvLyBMaW5lQnJlYWtl cjo6c2tpcExlYWRpbmdXaGl0ZXNwYWNlKCkgdG8gc2tpcCBvdmVyIHN1Y2ggd2hpdGVzcGFjZSBi ZWZvcmUgY2FsbGluZworICAgICAgICAgICAgLy8gQnJlYWtpbmdDb250ZXh0OjpoYW5kbGVUZXh0 KCkuCisgICAgICAgICAgICAvLyBUaGVyZWZvcmUsIHRoaXMgLTEgd3JhcHBpbmcgaXMgYmVuaWdu IChhbGJlaXQgYnJpdHRsZSkuCiAgICAgICAgICAgICBsaW5lTWlkcG9pbnRTdGF0ZS5taWRwb2lu dHMoKVt0cmFpbGluZ1NwYWNlTWlkcG9pbnRdLnNldE9mZnNldChsaW5lTWlkcG9pbnRTdGF0ZS5t aWRwb2ludHMoKVt0cmFpbGluZ1NwYWNlTWlkcG9pbnRdLm9mZnNldCgpIC0xKTsKIAogICAgICAg ICAvLyBOb3cgbWFrZSBzdXJlIGV2ZXJ5IHNpbmdsZSB0cmFpbGluZ1Bvc2l0aW9uZWRCb3ggZm9s bG93aW5nIHRoZSB0cmFpbGluZ1NwYWNlTWlkcG9pbnQgcHJvcGVybHkgc3RvcHMgYW5kIHN0YXJ0 cwo=