ucla-oarc-mobile / mwf

UCLA Mobile Web Framework
http://mwf.ucla.edu
Other
86 stars 25 forks source link

css.php Custom CSS Bug #159

Closed BerkeleyEdu closed 12 years ago

BerkeleyEdu commented 12 years ago

Hi,

The extra CSS which changes the header background to yellow is not loading in MWF 1.3 on qa:

http://m-qa.berkeley.edu/calday/locations.php

This is how the page should look (MWF 1.2) n prod:

http://m.berkeley.edu/calday/locations.php

Here is the css link:

-Sara

ebollens commented 12 years ago

Sara, I don't see this rule in your new CSS:

body.second-level h1#header {
    background: url("http://berkeley.edu/calday/img/calday-app-secondary-bg.jpg") repeat-x scroll 0 0 #FDBA14;
    height: 50px;
}
BerkeleyEdu commented 12 years ago

Correct, as I said css.php is not pulling in the extra css in MWF 1.3.

If you compare:

 http://m-qa.berkeley.edu/calday/header2.php  (using MWF 1.2, prod)

  <link rel="stylesheet" type="text/css" 

href="http://m.berkeley.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css" />

with

 http://m-qa.berkeley.edu/calday/header3.php  (using MWF 1.3, qa)

 <link rel="stylesheet" type="text/css" 

href="http://m-qa.berkeley.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css" />

You will see that the file calling css.php from the production server correctly merges the additional style sheet but the one using the new MWF 1.3 version does not load this same extra style sheet.

-Sara

On 8/30/2012 6:16 PM, Eric Bollens wrote:

Sara, I don't see this rule in your new CSS:

body.second-level h1#header { background: url("http://berkeley.edu/calday/img/calday-app-secondary-bg.jpg") repeat-x scroll 0 0 #FDBA14; height: 50px; }

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8179960.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Can you show me what is in your css.ini file and the directory structure under your CSS assets?

BerkeleyEdu commented 12 years ago

; MWF custom stylesheets configuration file ; ; @author ebollens ; @author trott ; @version 20111104

[css]

; Directories under assets/css that are loaded after the assets/css/default ; sheets. ; custom[]="ucb"

On 8/31/2012 1:39 PM, Eric Bollens wrote:

Can you show me what is in your |css.ini| file and the directory structure under your CSS assets?

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8204360.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

What's in your ucb directory?

BerkeleyEdu commented 12 years ago

-sh-3.2$ pwd /var/www/html/mwf/root/assets/css/ucb -sh-3.2$ ls -l total 28 -rwxrwxr-x 1 app_relmgt webmaster 2733 Aug 14 14:50 basic.css -rwxrwxr-x 1 app_relmgt webmaster 5713 Aug 14 14:55 full.css -rwxrwxr-x 1 app_relmgt webmaster 3611 Aug 14 12:05 standard.css

On 8/31/2012 1:51 PM, Eric Bollens wrote:

What's in your |ucb| directory?

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8204655.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

BerkeleyEdu commented 12 years ago

I created another test file using m.ucla.edu -- and same problem not loading extra css:

http://m-qa.berkeley.edu/calday/header4.php

http://m.ucla.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css view-source:http://m.ucla.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css

On 8/31/2012 1:51 PM, Eric Bollens wrote:

What's in your |ucb| directory?

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8204655.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Sara, I looked in the BerkeleyEdu repo and I don't see these commits anywhere. Can you create a branch and push them so that I can see your configuration?

BerkeleyEdu commented 12 years ago

I'm working on my maintenance branch and have not committed anything yet in case my MWF 1.3 work goes south. Should I commit this work-in-progress? Not sure what you mean by create a new branch?

-Sara

On 8/31/2012 3:01 PM, Eric Bollens wrote:

Sara, I looked in the BerkeleyEdu repo and I don't see these commits anywhere. Can you create a branch and push them so that I can see your configuration?

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8206160.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

If you're in a repo, you can issue something like this:

git checkout -b BerkeleyEdu/develop-1.3
git push origin BerkeleyEdu/develop-1.3

This will push it to GitHub but under a different branch so that you don't trample on your maint branch.

BerkeleyEdu commented 12 years ago

Sorry to be dense but once I really screwed things up in git. Here is what I have done so far for my MWF1.3 update

git checkout master ... pull upstream master ... git checkout -b BerkeleyEdu/maint BerkeleyEdu/master

git merge --no-ff master ... create mode...

-sh-3.2$ git checkout -b BerkeleyEdu/maint BerkeleyEdu/master Switched to a new branch 'BerkeleyEdu/maint'

-sh-3.2$ git merge --no-ff master ... a bunch of conflicts which I fixed

then tons of edits which I have NOT committed yet committed to 'BerkeleyEdu/maint'

so if I create a new branch and push, how do I get back to 'BerkeleyEdu/maint' since it was new and never committed? I suppose I could stay in "|BerkeleyEdu/develop-1.3|" ?

-Sara

On 8/31/2012 3:15 PM, Eric Bollens wrote:

If you're in a repo, you can issue something like this:

git checkout -b BerkeleyEdu/develop-1.3 git push origin BerkeleyEdu/develop-1.3

This will push it to GitHub but under a different branch so that you don't trample on your maint branch.

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8206425.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Sara,

You can switch to the maint branch with git checkout BerkeleyEdu/maint.

However, keep in mind where you commit things. Maintenance changes should be committed when you're on the BerkeleyEdu/maint branch, and then stuff related to 1.3 should be committed to your new branch you check out and push.

Looking at your command list, though, when you merged the latest pull of master into BerkeleyEdu/maint, you were updating BerkeleyEdu/maint to MWF 1.3. That means honestly that your maint branch is in fact the branch that I'm interested in that's about MWF 1.3 update.

My thought is just push the maint branch and we'll work on this one. No need to make another since you're using it as what I was describing as the 1.3 dev branch.

BerkeleyEdu commented 12 years ago

commit and push just push with no commit?

On 8/31/2012 3:36 PM, Eric Bollens wrote:

Sara,

You can switch to the maint branch with |git checkout BerkeleyEdu/maint|.

However, keep in mind where you commit things. Maintenance changes should be committed when you're on the |BerkeleyEdu/maint| branch, and then stuff related to 1.3 should be committed to your new branch you check out and push.

Looking at your command list, though, when you merged the latest pull of master into |BerkeleyEdu/maint|, you were updating |BerkeleyEdu/maint| to MWF 1.3. That means honestly that your maint branch is in fact the branch that I'm interested in that's about MWF 1.3 update.

My thought is just push the maint branch and we'll work on this one. No need to make another since you're using it as what I was describing as the 1.3 dev branch.

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8206785.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Since this is a 1.3 branch anyways, you can push it without affecting your master branch (or the develop branch even). Therefore, commit what you've got and let me look at it. As it stands now, I really can't be of much more help unless I can see the implementation.

BerkeleyEdu commented 12 years ago

OK, done. Thanks for your patience!

-sh-3.2$ git commit -m "Update to MWF 1.3 -- in progress" [BerkeleyEdu/maint be96c5e] Update to MWF 1.3 -- in progress Committer: app_relmgt app_relmgt@as-mobile-qa.IST.Berkeley.EDU Your name and email address were configured automatically based on your username and hostname. Please check that they are accurate. You can suppress this message by setting them explicitly:

 git config --global user.name "Your Name"
 git config --global user.email you@example.com

After doing this, you may fix the identity used for this commit with:

 git commit --amend --reset-author

-sh-3.2$ git push Enter passphrase for key '/home/app_relmgt/.ssh/id_rsa': Counting objects: 914, done. Compressing objects: 100% (340/340), done. Writing objects: 100% (711/711), 426.67 KiB, done. Total 711 (delta 412), reused 554 (delta 332) To git@github.com:BerkeleyEdu/mwf.git d080aa1..be96c5e BerkeleyEdu/maint -> BerkeleyEdu/maint bc4743a..52fff7b master -> master

On 8/31/2012 3:45 PM, Eric Bollens wrote:

Since this is a 1.3 branch anyways, you can push it without affecting your master branch (or the develop branch even). Therefore, commit what you've got and let me look at it. As it stands now, I really can't be of much more help unless I can see the implementation.

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8206942.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Sara, when I checked the file, it seems to be working just fine. I checked this via the page mwf/about.php as your index.php file committed is throwing major errors (and looks to be pretty different from the original index.php file).

The following definition propagated correctly:

#header {
  background: #383A7A url(img/berkeley_header-bg.png) repeat-x;
  height: 35px;
  border-bottom: 3px solid #EBB71B;
}

Therefore, looks to me like your css.ini configuration file is correct.

As for your calday/header4.php file, you're including m.ucla.edu's assets for some reason...

BerkeleyEdu commented 12 years ago

Yes, the LOCAL css is loading fine. It's when I try to load EXTERNAL css using "css.php", that it does not work. The file I am trying to load is: http://berkeley.edu/calday/css/mobile/basic.css

I set up header4.php to demonstrate that the external css files do NOT load using your copy of "css.php" either.

but the same extra file LOADS if I use the MWF 1.2 assets/css.php on my production site:

<link rel="stylesheet" type="text/css" href="http://m.berkeley.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css <view-source:http://m.berkeley.edu/assets/css.php?basic=http%3A%2F%2Fberkeley.edu%2Fcalday%2Fcss%2Fmobile%2Fbasic.css>" />

On 8/31/2012 5:00 PM, Eric Bollens wrote:

Sara, when I checked the file, it seems to be working just fine. I checked this via the page |mwf/about.php| as your |index.php| file committed is throwing major errors (and looks to be pretty different from the original |index.php| file).

The following definition propagated correctly:

header {

background: #383A7A url(img/berkeley_header-bg.png) repeat-x; height: 35px; border-bottom: 3px solid #EBB71B; }

Therefore, looks to me like your |css.ini| configuration file is correct.

As for your |calday/header4.php| file, you're including m.ucla.edu's assets for some reason...

— Reply to this email directly or view it on GitHub https://github.com/ucla/mwf/issues/159#issuecomment-8208078.

Sara Leavitt, Electronic Communications Specialist UC Berkeley Office of Public Affairs 2200 Bancroft Way, Berkeley, CA 94720-4204 Phone: 510 643-6163 http://events.berkeley.edu http://www.berkeley.edu/mobile

ebollens commented 12 years ago

Ah, that explains the confusion.

Looks like I tracked down the problem.

Change the last block of code in your assets/css.php file to this:

/**
 * Load URL-specified CSS files (minified) based on user agent.
 */
foreach ($classifications as $classification => $is_classification) {
    if ($is_classification && isset($_GET[$classification])) {
        foreach (explode(' ', $_GET[$classification]) as $file) {
            if (Path_Validator::is_safe($file, 'css') && $contents = Path::get_contents($file)) {
                echo ' ' . CSSMin::minify($contents);
            }
        }
    }
}   

Let me know if that solves the problem. Looks like there was a bug in the loop. Thanks for catching this! Looks like the JS handler is implemented differently so it shouldn't be a problem there.

The commit that broke this is here: 1cd93e63b3bc98261c7fd767dff4e0f79c06dee9 ... @Trott any reason to not make the change I listed above, as you were the committer of this change?

Trott commented 12 years ago

Totally looks like a bug in my refactoring to me. I'd recommend adding a PHPUnit test for it as well.

Trott commented 12 years ago

Actually, it would probably be a Cuke rather than a PHPUnit test, but you know what I mean...a test! :)

BerkeleyEdu commented 12 years ago

Fixed! And assume that this is the code you rolled into the latest release. Glad we got to the bottom of this!

On 8/31/12 6:00 PM, Eric Bollens wrote:

/**

  • Load URL-specified CSS files (minified) based on user agent. */ foreach ($classifications as $classification => $is_classification) { if ($is_classification && isset($_GET[$classification])) { foreach (explode('', $_GET[$classification]) as $file) { if (Path_Validator::is_safe($file, 'css') && $contents = Path::get_contents($file)) { echo '' . CSSMin::minify($contents); } } } }
ebollens commented 12 years ago

Yep, it was rolled into the latest release. Thanks for reporting it!