New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 591664 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Not on Chrome anymore
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

‘MakeMy Trip Login’ button doesn't seen properly on http://www.makemytrip.com/

Reported by dchau...@etouch.net, Mar 3 2016

Issue description

Chrome Version: 50.0.2661.11 (Official Build) d99fa8d200178bce07b02a15d79512e7236088ee-refs/branch-heads/2661@{#51} 32/64-bit.
OS: Mac

What steps will reproduce the problem?
1. Launch chrome and go to http://www.makemytrip.com/
2. Click on ‘Login my account’ button and then click on ‘Forgot Password’ link.
3. Now enter the valid ‘Email Id’ in textbox and click on submit button and observe.

‘MakeMy Trip Login’ button is not seen properly.
‘MakeMy Trip Login’ button should seen properly.

This is a non-regression issue seen from M-30 series.

Note: 
1. This issue is not seen on ‘Windows’ OS, will soon update the behaviour of ‘Linux’ OS.
2. This is working fine in Mozilla Firefox browser.

Kindly review the attached screencast for reference.
 
Actual_Behaviour.mov
9.2 MB Download
Status: Untriaged (was: Unconfirmed)
Untriaging it so that it gets addressed.
Above issue is not reproducible on 'Linux' OS.
Labels: OS-Linux
Labels: -OS-Linux

Comment 5 by rtoy@chromium.org, Mar 4 2016

Components: -Blink Blink>Paint
Labels: Needs-Feedback
Are you referring to the grey color of the text, that you expect to be blue?

The style for classes .chf_primary_btn, a.chf_primary_btn, in ch.css, includes "color: #fefeff !important;" which is overriding the colors specified elsewhere.

Comment 7 by dchau...@etouch.net, Mar 10 2016

Labels: -Needs-Feedback
With response to comment #6: Text color should be white and background color should be red.

Kindly review the attached screen-shot for reference.
Buttons_Screenshot.png
45.8 KB View Download
Owner: chrishtr@chromium.org
Status: Assigned (was: Untriaged)
I can reproduce and have saved a local copy to reduce it.
Cc: pdr@chromium.org
Reduced testcase:

<!doctype HTML>

<a id="target" onclick="toggleColor()" style="-webkit-appearance: button">test</a>

<script>
function toggleColor() {
    console.log("toggleColor() called")
    target.style.background = "red"
}
</script>

Clicking the link does not turn it red unless you remove -webkit-appearance: button.

This is almost certainly yet another corner case of the textDecorationOrColorChanged optimization in LayoutObject::adjustStyleDifference. There is text, but it doesn't appear that way to the code
because of -webkit-appearance.

Comment 10 by pdr@chromium.org, Mar 11 2016

Owner: esprehn@chromium.org
I think this is a regression from https://src.chromium.org/viewvc/blink?view=rev&revision=169408, as the issue is we don't cache the ua style in cacheUserAgentBorderAndBackground.

Comment 11 by pdr@chromium.org, Mar 11 2016

@esprehn, I didn't bisect back to confirm your patch caused this, but found that the cached ua style was not being set so it seems likely.
Cc: meade@chromium.org nainar@chromium.org
Components: -Blink>Paint Blink>CSS
Owner: ----
Status: Available (was: Assigned)
Yeah this is because cacheUserAgentBorderAndBackground() checks:

    if (!style()->hasAppearance())
        return;

but the -webkit-appearance from an author rule won't get applied until below that call in StyleResolver::applyMatchedProperties.

I think the fix is to just make webkit-appearance into a high priority property, which it should be, since other properties depend on its value.

That's a simple change, just move it before zoom in Source/core/css/CSSProperties.in
Hmm... Safari behaves the same as we do. Their restrictions on appearance as even more restricted, see: 
https://github.com/WebKit/webkit/blob/52b2434/Source/WebCore/css/StyleResolver.cpp#L1294
https://github.com/WebKit/webkit/blob/52b2434/Source/WebCore/css/StyleResolver.cpp#L1110

where they have a hard coded list of element names:

    return localName == HTMLNames::inputTag
        || localName == HTMLNames::textareaTag
        || localName == HTMLNames::buttonTag
        || localName == HTMLNames::progressTag
        || localName == HTMLNames::selectTag
        || localName == HTMLNames::meterTag
        || localName == HTMLNames::isindexTag;

given that appearance is poorly specced for values that are not "none", I kind of think we probably shouldn't fix this since at least our crazy is sort of in line with WebKit's.

meade@ and nainar@, what's your feeling here?
Labels: Hotlist-Interop
Owner: nainar@chromium.org
Status: Started (was: Available)
Hi @esprehn, 

If its as easy a fix as you say - why not? It seems like a developer pain. Though it is Mac only. Can't repro on Linux. 

I'll make the change.
Why is this an interop issue? "-webkit-appearance: button" is non-standard and #13 says we behave the same as Safari.
Labels: -Hotlist-Interop
Fair enough, my bad.
Status: WontFix (was: Started)
Closing bug as WontFix -> as per verbal discussion with @timloh, better to match Safari given -webkit-appearance is non standard. 
For context, my understanding is:

- On certain form elements (those where html.css specifies -webkit-appearance), setting certain properties (e.g. background-color on <button>) disables the native styling since we don't know how to apply these styles with the native look.
- If you set (e.g.) "-webkit-appearance: button; background: red;" on the any of these form elements, we won't use the native styling. Setting a button appearance on a <button> should be a no-op and setting a button appearance on a <progress> is just silly. So the behaviour seems sensible to me.
- If you set (e.g.) "-webkit-appearance: button; background: red;" on any other element, we use the native look (we have to pick between native look or red background). I would guess the behaviour wasn't really thought about but just accidental. It makes -webkit-appearance behave differently on form elements to other elements, but...
- Chrome and Safari current behave identically in all of these behaviours. The hard-coded list mentioned in comment #13 is exactly the previously mentioned form elements. So changing anything here is just going to make more browsers more inconsistent.

Sign in to add a comment