Opened 3 years ago

Closed 3 years ago

#13155 closed Bug (fixed)

[Lineutils, DnD] incorrenct left margin in manual tests

Reported by: Piotr Jasiun Owned by: Olek Nowodziński
Priority: Normal Milestone: CKEditor 4.4.8
Component: General Version:
Keywords: Cc:

Description (last modified by Piotr Jasiun)

Because manual tests use left margin on the body element, lineutils have incorrect left margin.

To reproduce:

Magicline looks good in manual tests.

Attachments (2)

Screen Shot 2015-05-11 at 16.58.38.png (299.4 KB) - added by Piotrek Koszuliński 3 years ago.
Screen Shot 2015-05-11 at 16.59.26.png (112.4 KB) - added by Piotrek Koszuliński 3 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 3 years ago by Piotr Jasiun

Description: modified (diff)

comment:2 Changed 3 years ago by Piotr Jasiun

Description: modified (diff)
Summary: Magicline and lineutils incorrenct left margin in manual testsLineutils incorrenct left margin in manual tests

comment:3 Changed 3 years ago by Piotr Jasiun

Related: #13156.

comment:4 Changed 3 years ago by Olek Nowodziński

/cc

comment:5 Changed 3 years ago by Olek Nowodziński

Status: newconfirmed
Summary: Lineutils incorrenct left margin in manual tests[Lineutils, DnD] incorrenct left margin in manual tests

comment:6 Changed 3 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:7 Changed 3 years ago by Olek Nowodziński

Status: assignedreview

Changes in branch:t/13155.

comment:8 Changed 3 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.5.0

I rebased this branch on master, because this is a bug fix that can go in 4.4.8. However, during tests on IEs I got many errors when moving mouse over the classic editor. See attached screenshots. (PS. I removed the milestone for now, because this isn't a regression and it should not block 4.5.0 or 4.4.8).

Changed 3 years ago by Piotrek Koszuliński

Changed 3 years ago by Piotrek Koszuliński

comment:9 Changed 3 years ago by Piotrek Koszuliński

Status: reviewreview_failed

comment:10 Changed 3 years ago by Piotrek Koszuliński

Hm... I've just checked that these errors are not regressions caused by this patch. Could you anyway check that this isn't something caused by the manual test itself or something that we can quickly fix?

comment:11 Changed 3 years ago by Olek Nowodziński

Well... yep.

comment:12 Changed 3 years ago by Olek Nowodziński

Milestone: CKEditor 4.5.0
Status: review_failedreview

Could you anyway check that this isn't something caused by the manual test itself or something that we can quickly fix?

Fixed. Also:

  • Found out that the fix in this branch broke the line positioning when body has position: static.
    • Fixed it.
    • Added manual test for such case.

Re-attaching to 4.5.

comment:13 Changed 3 years ago by Piotrek Koszuliński

Why 4.5.0? Cannot it land in 4.4.8?

comment:14 in reply to:  13 Changed 3 years ago by Olek Nowodziński

Replying to Reinmar:

Why 4.5.0? Cannot it land in 4.4.8?

Easier to rebase :D

comment:15 Changed 3 years ago by Piotrek Koszuliński

I've easily rebased the branch on 4.4.8:

I rebased this branch on master, because this is a bug fix that can go in 4.4.8.

comment:16 in reply to:  15 Changed 3 years ago by Olek Nowodziński

Milestone: CKEditor 4.5.0CKEditor 4.4.8

Replying to Reinmar:

I've easily rebased the branch on 4.4.8:

I rebased this branch on master, because this is a bug fix that can go in 4.4.8.

Alright. I'm cool with it then.

comment:17 Changed 3 years ago by Artur Delura

Rebased branch to the newest master.

comment:18 Changed 3 years ago by Piotrek Koszuliński

Status: reviewreview_failed

Hm... I've just checked that these errors are not regressions caused by this patch. Could you anyway check that this isn't something caused by the manual test itself or something that we can quickly fix?

On IE8 I still see the same errors.

comment:19 in reply to:  18 Changed 3 years ago by Olek Nowodziński

Status: review_failedreview

Replying to Reinmar:

Hm... I've just checked that these errors are not regressions caused by this patch. Could you anyway check that this isn't something caused by the manual test itself or something that we can quickly fix?

On IE8 I still see the same errors.

Pushed a commit to rebased branch:t/13155 which fixes the issue.

comment:20 Changed 3 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

Fixed on master with git:4b039c3.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy