On Tue, 12 Feb 2008 22:36:11 +0100, Tom Lazar
<[EMAIL PROTECTED]> wrote:
hi florian, hello fellow framework team members,
i started reviewing plip 212 on the plane back to berlin and got most
covered. i committed my initial review and post a copy of it here, for
your convenience. i still need to repeat the click tests i've done with
IE 6 and IE 7, as i haven't got Parallels installed on my macbook (yet).
also, i intend to look at the js code changes and migration stuff, but
judging from my previous experiences with florian's code i don't expect
any snags there. however, i did run into a few problems with this
buildout and currently don't think it's ready for merging, see my notes
below. i will continue the review tomorrow evening, along with my other
outstanding reviews.
Hi!
Thanks for the review. I just wanted to make clear that I didn't write any
of this code, I only wrote the PLIP, did the review buildout and foxed
some small things. All the heavy lifting was done by Martijn Pieters. I
will give some comments below.
--snip--
Note, all tests have been conducted with jquery 1.2.2 which is part of
the bundle, as well as with version 1.2.3 which is currently the latest
revision of jquery.
= manual click tests
performed with fresh checkout, new instance with Plone Default skin.
Using Firefox 2.0.0.12 and Safari Version 3.0.4 (5523.15), Win IE 6 + 7
tbd. Features and areas covered:
* livesearch, portal calendar pagination, external links, inline
editing, folder_contents reordering:
no problems encountered, everything worked as expected.
* highlighting of search results
This didn't work on any browser. Calling a url such as
http://localhost:8080/jquery/front-page?searchterm=comfortable would
not actually highlight the word (eventhough it exists in the default
front page)
instead i got the following error in jquery.js (via firebug)
a is not a function
[Break on this error] eval(function(p,a,c,k,e,r){e=function(c)
{return(c<a?'':e(parseInt(c/a)))+((c=c%a...
Note: this error remained, even after replacing the shipping jquery
version 1.2.2 with the meanwhile current version 1.2.3
This looks like a syntax error in that file. I will look into it.
Additional errors:
the display menu wasn't working as expected, i.e. the click action on
the display menu was not caught but instead resulted in a page load of
the `select_default_view`, however, this only occurred with jquery
1.2.3, 1.2.2 worked fine.
workflow actions however didn't work in neither 1.2.2 nor 1.2.3, i.e.
publishing resulted in a page load instead of just a viewlet refresh.
This is either another syntax error, or some issue with the KSS
interaction.
= running unit tests =
running all tests (./bin/instance/test -v -v -s plone) produced the
following result:
Tests with failures:
test_published_news_items
(plone.app.portlets.tests.test_events_portlet.TestRenderer)
test_published_news_items
(plone.app.portlets.tests.test_news_portlet.TestRenderer)
/opt/zope/buildout/eggs/plone.app.workflow-1.0.1.1-py2.4.egg/plone/
app/workflow/tests/onestateworkflow.txt
Total: 1079 tests, 3 failures, 0 errors
I didn't look into this at all, because we didn't change anything besides
JS code and some registrations in JS registry. I will look into upgrading
the buildout to the latest Plone version if it isn't already.
= code review =
to be done (check migration, look into the js code changes)
Regards,
Florian Schulze
_______________________________________________
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
