Opened 7 years ago

Closed 7 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 7 years ago.
4521_2_ref.patch (1.2 KB) - added by tobiasz.cudnik 7 years ago.
4521_2.patch (834 bytes) - added by garry.yao 7 years ago.
4521_2_-_IE8_Issue.png (34.9 KB) - added by fredck 7 years ago.
4521_3.patch (727 bytes) - added by garry.yao 7 years ago.
4521_4.patch (8.3 KB) - added by garry.yao 7 years ago.
4521_5.patch (10.0 KB) - added by garry.yao 7 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 Changed 7 years ago by fredck

  • Keywords Confirmed added
  • Priority changed from High to Normal
  • Version set to CKEditor 3.0

Confirmed with CKEditor 3.0.

Changed 7 years ago by garry.yao

comment:2 Changed 7 years ago by garry.yao

  • Keywords Review? added
  • Owner set to garry.yao
  • Status changed from new to assigned

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 7 years ago by tobiasz.cudnik

comment:3 Changed 7 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 7 years ago by garry.yao

comment:4 Changed 7 years ago by garry.yao

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

comment:5 Changed 7 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 7 years ago by fredck

  • 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 7 years ago by garry.yao

  • Keywords Review? added; Review- removed

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

Changed 7 years ago by fredck

comment:8 Changed 7 years ago by fredck

  • 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 7 years ago by garry.yao

comment:9 Changed 7 years ago by garry.yao

  • Keywords Review? added; Review- removed

Confirmed, delay the calculation a bit resolves this issue.

comment:10 Changed 7 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 7 years ago by garry.yao

comment:11 Changed 7 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 7 years ago by fredck

  • 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 7 years ago by fredck

Btw... great job really!

Changed 7 years ago by garry.yao

comment:14 Changed 7 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 7 years ago by fredck

  • Keywords Review+ added; Review? removed

comment:16 Changed 7 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [4668].

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