Bug 38697

Summary: REGRESSION (r58299): Replying on reddit.com no longer works
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: CSSAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: hyatt, jalada+bugzilla, mitz, s+webkit, visnupx, yuzo
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
reduced test case
none
naive fix ggaren: review+

Description Alexey Proskuryakov 2010-05-06 15:14:03 PDT
STEPS TO REPRODUCE:
1) Open http://www.reddit.com/r/AskReddit/comments/a7pud/whats_your_favorite_ted_talk/
2) Click the "Reply" link (you might need to be logged in for this).

EXPECTED RESULTS:
A text area opens for you to write a comment

ACTUAL:
It doesn't.

Alternatively (if you don't have a reddit account), just click "Register" link.

<rdar://problem/7931177>
Comment 1 Alexey Proskuryakov 2010-05-06 15:15:07 PDT
This happens because document.querySelectorAll(".login-popup:first") no longer raises an exception.
Comment 2 Alexey Proskuryakov 2010-05-06 15:17:50 PDT
Created attachment 55306 [details]
reduced test case
Comment 3 Alexey Proskuryakov 2010-05-06 16:35:09 PDT
I think that this goes both ways. The new first, left and right classes are allowed outside @page rules, but also, other classes are allowed inside page rules, but they shouldn't. 

Correct (from the spec):

page :
       PAGE_SYM S* IDENT? pseudo_page? S* 
       '{' S* [ declaration | margin ]? [ ';' S* [ declaration | margin ]? ]* '}' S*
       ;

pseudo_page :
       ':' [ "left" | "right" | "first" ]
       ;

Wrong (CSSGrammar.y):

page:
    PAGE_SYM maybe_space page_selector maybe_space
...
page_selector:
    IDENT {
...
    | IDENT pseudo {
...
    | pseudo {
...
    | /* empty */ {

Note that any pseudo is allowed in page rules by the grammar here, not just pseudo_page.
Comment 4 Alexey Proskuryakov 2010-05-06 16:42:25 PDT
Created attachment 55318 [details]
naive fix
Comment 5 Geoffrey Garen 2010-05-06 16:50:02 PDT
Comment on attachment 55318 [details]
naive fix

r=me
Comment 6 Alexey Proskuryakov 2010-05-06 17:04:39 PDT
Committed <http://trac.webkit.org/changeset/58922>. Add a FIXME comment, as suggested by Mitz.
Comment 7 Darin Adler 2010-05-06 17:14:49 PDT
Comment on attachment 55318 [details]
naive fix

> +    document.querySelectorAll(".login-popup:first");

I'd be happier if you included test cases for all three of the affected pseudo classes.
Comment 8 Yuzo Fujishima 2010-05-06 18:04:39 PDT
Sorry for the regression and thank you for the fix.
Comment 9 Yuzo Fujishima 2010-05-06 18:07:19 PDT
I'll address the grammar issue under a separate bug.
Comment 10 Alexey Proskuryakov 2010-05-07 12:17:15 PDT
*** Bug 38729 has been marked as a duplicate of this bug. ***
Comment 11 visnu pitiyanuvath 2010-05-07 15:06:36 PDT
*** Bug 38499 has been marked as a duplicate of this bug. ***
Comment 12 Alexey Proskuryakov 2010-05-09 16:47:57 PDT
*** Bug 38804 has been marked as a duplicate of this bug. ***