Bug 186640 - [LFC] Fix static position left/top
Summary: [LFC] Fix static position left/top
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-06-14 18:50 PDT by zalan
Modified: 2018-06-18 00:53 PDT (History)
9 users (show)

See Also:


Attachments
Patch (4.86 KB, patch)
2018-06-14 19:38 PDT, zalan
koivisto: review+
ltilve+ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ltilve-gtk-wk2-ews for gtk-wk2 (2.79 MB, application/zip)
2018-06-15 04:52 PDT, Igalia-pontevedra EWS
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2018-06-14 18:50:08 PDT
return { top, left }; -> return { left, top };

This is so error prone and should find a way to enforce left/top vs. top/left.
Comment 1 zalan 2018-06-14 19:38:41 PDT
Created attachment 342785 [details]
Patch
Comment 2 Igalia-pontevedra EWS 2018-06-15 04:52:09 PDT
Comment on attachment 342785 [details]
Patch

Attachment 342785 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.webkit.org/results/8195225

New failing tests:
http/tests/misc/char-encoding-bocu-1-blacklisted.html
Comment 3 Igalia-pontevedra EWS 2018-06-15 04:52:16 PDT
Created attachment 342802 [details]
Archive of layout-test-results from ltilve-gtk-wk2-ews for gtk-wk2

The attached test failures were seen while running run-webkit-tests on the gtk-wk2-ews.
Bot: ltilve-gtk-wk2-ews  Port: gtk-wk2  Platform: Linux-4.16.0-0.bpo.1-amd64-x86_64-with-debian-9.4
Comment 4 Antti Koivisto 2018-06-15 08:08:00 PDT
Comment on attachment 342785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342785&action=review

> Source/WebCore/ChangeLog:8
> +        In visual formatting model, we normally go like [top, left] while LayoutPoint takes [x, y]. Let's make this less error prone.

Couldn't we use [left, top] [x, y] consistently?
Comment 5 Antti Koivisto 2018-06-15 08:29:59 PDT
Comment on attachment 342785 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342785&action=review

> Source/WebCore/layout/FormattingContext.h:86
> +            LayoutUnit y; // top
> +            LayoutUnit x; // left

Maybe we could have something like LayoutUnit<Vertical> and LayoutUnit<Horizontal> so mixing would require explicit conversions?
Comment 6 zalan 2018-06-15 08:49:08 PDT
(In reply to Antti Koivisto from comment #5)
> Comment on attachment 342785 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=342785&action=review
> 
> > Source/WebCore/layout/FormattingContext.h:86
> > +            LayoutUnit y; // top
> > +            LayoutUnit x; // left
> 
> Maybe we could have something like LayoutUnit<Vertical> and
> LayoutUnit<Horizontal> so mixing would require explicit conversions?
Great idea. Yeah, we should do something like that.
Comment 7 zalan 2018-06-15 09:26:28 PDT
Committed r232876: <https://trac.webkit.org/changeset/232876>
Comment 8 Radar WebKit Bug Importer 2018-06-15 09:27:20 PDT
<rdar://problem/41161567>