Opened 8 years ago

Closed 8 years ago

#4521 closed Bug (fixed)

[IE] Tab key to dialog buttons shift their position

Reported by: Garry Yao Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.1
Component: UI : Dialogs Version: 3.0
Keywords: IE Confirmed Review+ Cc:

Description

Environments

IE7 Standards mode

Procedures

  1. Open 'link' dialog;
  2. Tab through the dialog fields until reach 'Ok' button;
  3. Actual result:
    • All buttons position at the bottom of dialog now shift left.

Attachments (7)

4521.patch (1.6 KB) - added by Garry Yao 8 years ago.
4521_2_ref.patch (1.2 KB) - added by Tobiasz Cudnik 8 years ago.
4521_2.patch (834 bytes) - added by Garry Yao 8 years ago.
4521_2_-_IE8_Issue.png (34.9 KB) - added by Frederico Caldeira Knabben 8 years ago.
4521_3.patch (727 bytes) - added by Garry Yao 8 years ago.
4521_4.patch (8.3 KB) - added by Garry Yao 8 years ago.
4521_5.patch (10.0 KB) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added
Priority: HighNormal
Version: CKEditor 3.0

Confirmed with CKEditor 3.0.

Changed 8 years ago by Garry Yao

Attachment: 4521.patch added

comment:2 Changed 8 years ago by Garry Yao

Keywords: Review? added
Owner: set to Garry Yao
Status: newassigned

The culprit here is the 'display:inline-table' applied on the footer table which cause IE7 to be buggy, I had to say the coming patch is really a quick dirty fix ( with hard-coding border sizes ) which use another approach to right align the footer.

A decent fix could relate to the fixing of #4174.

Changed 8 years ago by Tobiasz Cudnik

Attachment: 4521_2_ref.patch added

comment:3 Changed 8 years ago by Tobiasz Cudnik

Attaching reference patch which works-around this issue in another way. Works with other dialogs in Kama skin, could have issues in other skins.

Changed 8 years ago by Garry Yao

Attachment: 4521_2.patch added

comment:4 Changed 8 years ago by Garry Yao

@Tobias, with the patch I could still saw the shifting.

comment:5 Changed 8 years ago by Garry Yao

After some investigation I found a even more simple fix, that the shift will no longer happen once we give the wrapper div (.cke_dialog_footer) a fixed size, attaching a new patch.

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I'm testing it with IE8 in compat mode and the footer size is not calculated properly. There should be some CSS magic for it.

comment:7 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

I'm having it working perfectly with IE8 compact, which dialog specifically?

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 4521_2_-_IE8_Issue.png added

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I've attached a screenshot of the issue.

An additional note... it happens only when loading the sample page in a clean cache IE8.

Changed 8 years ago by Garry Yao

Attachment: 4521_3.patch added

comment:9 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

Confirmed, delay the calculation a bit resolves this issue.

comment:10 Changed 8 years ago by Garry Yao

Keywords: Review? removed

Plz hold with the review, I'm composing something that could fix both this and #4174.

Changed 8 years ago by Garry Yao

Attachment: 4521_4.patch added

comment:11 Changed 8 years ago by Garry Yao

Keywords: Review? added

I'm proposing a new dialog layout (theme) based on table structure, which benefits from table cell's scalable size natural to well emulate the max/min style (we'd ever introduced at #3878) for all unsupported browsers (IE6/7).
The result is pretty in expectation, that we completely forget about dialog scrollbar issues (#4174,#4540,#3743,#3787,#3962,#4122) and it appears to eliminate many weirds from dialog system, e.g. this bug and #4122.
The patch should be well tested in all browsers of all modes with all skins, though I've tested it exclusively already.
After closing this ticket, we should begin to look for swiping those hacks that aren't necessary anymore in existing dialog parts.

comment:12 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
  • Sometimes (on first call) the dialog open aligned to the top of the page in FF.
  • The v2 and office2003 dialogs have broken layout in FF. Ok in IE.
  • In browsers that support it, we could now use "min-width" instead of "width" for the dialog contents area. This makes the size calculation a bit better (in FF at least).

comment:13 Changed 8 years ago by Frederico Caldeira Knabben

Btw... great job really!

Changed 8 years ago by Garry Yao

Attachment: 4521_5.patch added

comment:14 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

As discussed with Fred:

Sometimes (on first call) the dialog open aligned to the top of the page in FF.

Confirmed with Fred it's pretty hard to reproduce, besides it probably not brought by this patch.

The v2 and office2003 dialogs have broken layout in FF. Ok in IE

Fixed with new patch.

In browsers that support it, we could now use "min-width" instead of "width" for the dialog contents area. This makes the size calculation a bit better (in FF at least).

We'll need to introduce browser sniffing again, and it's hard to guarantee that every browser agree with the same thing, let's leave it for later investigation.

comment:15 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:16 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [4668].

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