Bug 63573 – REGRESSION (r88415): Range slider doesn't have focus appearance.

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Alexey Proskuryakov]

Created attachment 98982 [details]
slider

Steps to reproduce:
1. Open attached slider.html in Safari.
2. Assuming default user preferences, press Option+Tab several times.

Expected behavior: it should be focused.
Actual behavior: it's not.

Unfortunately, this is the smallest range of revisions I can make. Does someone remember any suspicious changes at that time?

[Kent Tamura]

My changes for tyep=range:

r88294 r=kling [Bug 62207] [Qt] RenderThemeQt::adjustSliderThumbSize() should not refer to the parent style
r88301 r=rniwa [Bug 62195] Convert fast/forms/thumbslider-crash.html to dumpAsText
r88405 r=dglazkov [Bug 62208] Change the argument of RenderTheme::adjustSliderThumbSize(): RenderObject* -> RenderStyle*
r88415 r=dglazkov [Bug 62196] Allow drawing a slider thumb for any nodes

Some focus-related changes in the range:
http://trac.webkit.org/changeset/88418 Makes sure that document.activeElement won't be an element in shadow root.
http://trac.webkit.org/changeset/88421 A forward/backward tab traversal now visits focusable elements in a shadow root.

r88415 and r88421 are suspicious.  I'll investigate.

[Kent Tamura]

(In reply to comment #0)
> Expected behavior: it should be focused.
> Actual behavior: it's not.

Actually it has focus, but has no focus appearance, doesn't it?
Keyboard operation is very slow in this test case because of a wide value range and the default step 1.

[Alexey Proskuryakov]

Looks like that!

[Kent Tamura]

Created attachment 99213 [details]
Patch

[Kent Tamura]

Comment on attachment 99213 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests. This behavior strongly depends on the local computer setting.

Oops, no.  We can make a slider focus regardless of the setting.

[Kent Tamura]

Created attachment 99217 [details]
Patch 2

[Alexey Proskuryakov]

Comment on attachment 99217 [details]
Patch 2

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

> LayoutTests/fast/forms/input-appearance-range.html:7
>     - three horizontal sliders with various thumb positions.
> +     - The second slider has focus.
>     - a vertical slider

Indentation looks strange here.

> Source/WebCore/rendering/RenderThemeMac.mm:1321
> +    updateFocusedState(sliderThumbCell, o->node() && o->node()->focusDelegate()->renderer() ? o->node()->focusDelegate()->renderer() : o);

Can you put parentheses around the ternary operator condition for clarity? Some compilers might actually produce a warning otherwise.

[Kent Tamura]

(In reply to comment #7)

Thank you for reviewing! I'll land with fixes you pointed out.

[Kent Tamura]

Committed r90436: <http://trac.webkit.org/changeset/90436>