New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

www.suntory.co.jp - mobile site is not usable #1008

Closed
miketaylr opened this Issue Apr 28, 2015 · 17 comments

Comments

Projects
None yet
5 participants
@miketaylr
Copy link
Member

miketaylr commented Apr 28, 2015

URL: http://www.suntory.co.jp/
Browser / Version: Firefox 40.0
Operating System: Android 4.whatever
Problem type: Mobile site is not usable

Steps to Reproduce

  1. Navigate to: http://www.suntory.co.jp/

Expected Behavior:
Layout should be decent. Video should play.

Actual Behavior:
There's a few JS errors in the console to be investigated:

ReferenceError: modExp is not defined movieControl_sp.js:57:1
TypeError: ft_menu_btn is null common_parts.js:315:1
ReferenceError: onTouchStart is not defined jquery.flexslider.js:397:12

country: jp

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

ReferenceError: onTouchStart is not defined jquery.flexslider.js:397:12 aww, I know this one. I have a fork that fixes it--but need to try to get the PR merged into master (I think my pull request is over a year old 😢).

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

Most of the layout issues are caused by not having unprefixed -webkit-background-position in http://mobile.suntory.co.jp/common/css/common.css. index.css is also missing the same thing (except for the flexslider styles).

There are also a few instances where they try to use XUL box layout, e.g.,

#product_area ul {
display: -webkit-box;
display: -moz-box;
}

If they add the standard display: flex, the site appears as expected. That covers the layout stuff, pretty much.

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

For the onTouchStart is not defined problem, it's fixed by miketaylr/FlexSlider@0b54de9. I'm going to send an updated PR against FlexSlider master -- I see that the original bug has a 2.5 milestone attached to it (current release is 2.4).

So we'll need someone at suntory to update this plugin, or manually apply a patch.

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

I updated my PR against FlexSlider's latest master. woocommerce/FlexSlider#986

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

OK, talking with @hallvors -- it looks like they're using the old Brightcove player code which isn't very compatible with Firefox mobile browsers. So that explains the audio but no playing video. Updating to the new player should fix those issues for us.

See https://bugzilla.mozilla.org/show_bug.cgi?id=794100#c30 for more info on that.

@miketaylr

This comment has been minimized.

Copy link
Member Author

miketaylr commented Apr 28, 2015

And as for the TypeError: ft_menu_btn is null common_parts.js:315:1 error, Chrome Mobile also has that. So there's something equally broken here. The developers are likely interested, but I won't dig much further on that. It just seems like an element is missing.

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented May 19, 2015

Suntory is another mega corporation.
There is a @suntorydevteam on Github, but that doesn't mean that someone is listening and that it is related to the Suntory company.

There is a form which could be used maybe as a first step. We could try even if usually it's not that successful for technical issues.

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented May 19, 2015

Ah trying to get closer. Big corporation, maybe we could try to reach out through people working with them like Hakuhodo and/or people like Seiichi Okura. To think.

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented May 20, 2015

I found the bar-navi project online. It's a bit old but resides on the suntory domain name. The person who seems to have been working on it is @ackintosh

I wonder if @ackintosh if he reads his github notifications can help us to contact the right persons at Suntory, so they can fix their mobile Web site.

@karlcow karlcow added sitewait and removed needscontact labels Jun 19, 2015

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented Jun 19, 2015

@kudodo

This comment has been minimized.

Copy link

kudodo commented Jun 29, 2015

I've contacted Suntory. The person in charge of it requests a meeting with us. I'll send detail to @karlcow.

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented Nov 4, 2015

We have met them since that comment. A long time ago. With a team of their developers. They listened to us. I'm not sure how far they advanced in the project of fixing it.

I was testing today with

  • layout.css.prefixes.webkit;false
  • layout.css.unprefixing-service.enabled;false
  • layout.css.unprefixing-service.globally-whitelisted;false

The same issue are still there.

capture d ecran 2015-11-04 a 14 04 03

When I switch only (the others stay the same)

  • layout.css.prefixes.webkit;true

I get some fixes

capture d ecran 2015-11-04 a 14 08 20

But there are still issues for flexbox. For example, the orange 'NEW' and blue 'kcal' buttons are in boxes we should equally occupy the space. When I check with the inspector, I see:

capture d ecran 2015-11-04 a 14 09 33

If I replace this display: -moz-box by display: flex this is working properly. I wonder if @dholbert has an idea. (I'm testing with Nightly 45.0a1 (2015-11-03) )

@dholbert

This comment has been minimized.

Copy link

dholbert commented Nov 4, 2015

If I replace this display: -moz-box by display: flex this is working properly.

Cool. That's not a big surprise; display:-webkit-box is closer to display:flex than it is to display:-moz-box.

The functionality behind "layout.css.prefixes.webkit" isn't complete yet (and doesn't yet include -webkit-box support), so it's not surprising that that pref doesn't fix these flexbox issues at this point. However, earlier today I posted patches that should make this work, over in https://bugzilla.mozilla.org/show_bug.cgi?id=1208635 .

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented Nov 4, 2015

Ah Fabulous @dholbert Thanks!!! 🎅

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented Nov 6, 2015

And today in the latest nightly!

Screenshot of the site issue

The flexbox issue is "solved". ^_^ 🎉

@dholbert

This comment has been minimized.

Copy link

dholbert commented Nov 6, 2015

Hooray!

@karlcow

This comment has been minimized.

Copy link
Contributor

karlcow commented Feb 19, 2016

fixed by layout.css.prefixes.webkit;true

@karlcow karlcow added this to the worksforme milestone Oct 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment