Closed Bug 157210 Opened 22 years ago Closed 22 years ago

[FIX]renders input #2 like input #1 they have the same value, but different cases

Categories

(Core :: DOM: Core & HTML, defect, P1)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.3alpha

People

(Reporter: mattshep99, Assigned: bzbarsky)

References

Details

Attachments

(6 files)

testcase
191 bytes, text/html
Details
another testcase
659 bytes, text/html
Details
And yet another testcase
816 bytes, text/html
Details
Lest there be aspersions cast upon the parser
240 bytes, text/html
Details
Untested version 1
938 bytes, patch
jst
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
test for regressions
480 bytes, text/html
Details 
From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
BuildID:    2002053012

Open this HTML up in Mozilla or Netscape 6.2.  It will render the inputs the 
same (and submit the same value) even though the values differ by case.  Only 
happens if the 2 characters are 8 characters appart.

<html>
<body>
<form name="MainForm">

<input type="text" name="lower" value="abcdefghabcdefghi">
<input type="text" name="upper" value="AbcdefghAbcdefghi">

</form>
</body>

</html>




Reproducible: Always
Steps to Reproduce:
1. Open the HTML (in the description) up in Mozilla or Netscape 6.2
2.
3.

Actual Results:  It will render the inputs the same (and submit the same value) 
even though the values differ by case.  Only happens if the 2 characters are 8 
characters appart.


Expected Results:  It should have shown and submitted the values as 
lower=abcdefghabcdefgh and upper=AbcdefghAbcdefgh
confirming with 2002090208/trunk on win2k. very odd.
Assignee: sgehani → alexsavulov
Status: UNCONFIRMED → NEW
Component: XP Apps → Form Submission
Ever confirmed: true
QA Contact: paw → vladimire
Attached file testcase — 
reporter's HTML testcase
Attached file another testcase — 

    
    

    
  


  

    
    

    
  
OK... here's what's going on in this last testcase:

1)  The first input is created.  Its attrbute list is created.
2)  The second input is created.  Its atribute list is created.
3)  We call SetAttr on the first input.  All is peachy.
4)  We call SetAttr on the second input. This is where things break:
5)  nsGenericHTMLElement::SetAttr calls mAttributes->SetAttributeFor.
6)  The attribute in question ("value") has stylistic impact (a reflow), so
    aMappedToStyle is true.
7)  We set the attribute in mMapped
8)  We call UniqueMapped()
9)  We call aSheet->UniqueMappedAttributes()
10) We do the hash table lookup and the hashkey matches an existing entry (that
    of the attr list of the first input).  Note that the "name" attr is not
    mapped to style, so it's not part of the equation here; only "type" and
    "value" are.
11) We call MappedAttrTable_MatchEntry()
12) This calls attributes->Equals(entry->mAttributes, equal);
13) nsHTMLMappedAttributes::Equals walks the attribute lists comparing the
    attributes using HTMLAttribute's operator==
14) This compares the mBits member (via ==) and the mValue member via
    nsHTMLValue's operator==
15) nsHTMLValue's operator== is case-insensitive for strings (!)
16) We end up using the hashed attribute list.
17) We show the wrong thing

So a few possible issues here:

A)  Should operator== on nsHTMLValue be case-insensitive?
B)  If so, should HTMLAttribute use a different method to compare two
    nsHTMLValues
C)  If not, should there be a different method to compare two HTMLAttributes?

Unrelated issues:

I.  The code in nsHTMLAttributes::EnsureSingleMappedFor shouldn't be using
    Clone(); it would make a lot more sense to call a function that clones iff
    the mUseCount > 1.  Then in the "no sharing" case we won't spend a bunch of
    time creating and destroying attr lists.
II. jkeiser is not sure we need separate mUseCount and mRefCnt.
III. It would be nice to be able to do the hash lookup _without_ constructing a
     unique attr list...  that is, it would be good to have a something that
     takes an attr list and an attr to add and handles the hash lookup without
     modifying the attr list.

Thoughts on it all?
Severity: normal → major

    
    

    
  
for most attributes we want to do a case-insensitive compare, so that part is
correct. Actually what worries me most (other then the fact that we do a lot of
unneccesary work) is the fact that the value of the "value"-attribute is part of
the whole unique-attributes hash thing.

The value attribute should, as far as i can think of right now, not affect
styling in any other way then that it is the .defaultValue for the <input>
(which then decides the value of .value which then decides the value of the editor)

So the fix for this might be as easy as considering the "value" attribute as a
non-styled one

    
    

    
  
OK.  So the way we set the aMappedToStyle boolean is:

    GetMappedAttributeImpact(aAttribute, modHint, impact);
    aMappedToStyle = ((impact & ~(nsChangeHint_AttrChange | nsChangeHint_Aural
                                 | nsChangeHint_Content)) != 0);

The problem is that the mapped impact of "value" for inputs is
NS_STYLE_HINT_REFLOW.  So it's considered to be mapped to style.  So is the
"type" attribute, for the same reason.  And so on.

Perhaps we need to stop and think about why we have this mapped attribute table
to start with?  Does it really give us a footprint win?  If so, why are we only
storing "attributes mapped to style" in here?  It looks like we have
nsHTMLMappedAttributes implementing nsIStyleRule, but it only does this via
calling the parent content's attribute mapping function, so we should be able to
just hoist that out to nsHTMLAttributes...

    
    

    
  
I might have this fixed in my tree. Need to do a couple more tests

    
  
Priority: -- → P2

    
    

    
  


  
This should fix it. Still havn't tested it

    
    

    
  
Bulk moving P1-P5 un-milestoned bugs to future. 
Target Milestone: --- → Future

    
    

    
  
Bah.  Taking.  I'll try to write some tests and attach them, then aim for
beginning of 1.3a to land so we can iron out any fallout (which I do not expect).
Assignee: alexsavulov → bzbarsky
OS: Windows 2000 → All
Priority: P2 → P1
Hardware: PC → All
Summary: renders input #2 like input #1 they have the same value, but different cases → [FIX]renders input #2 like input #1 they have the same value, but different cases
Target Milestone: Future → mozilla1.3alpha

    
    

    
  

      
      
      
      

        Attached file
          
        test for regressions
        — 
      

    


  

    
    

    
  
Comment on attachment 97715 [details] [diff] [review]
Untested version 1

sr=bzbarsky.  It's the right approach, works good, doesn't regress things as
far as I can tell.

        Attachment #97715 -
        Flags: superreview+

    
    

    
  
Comment on attachment 97715 [details] [diff] [review]
Untested version 1

r=jst

        Attachment #97715 -
        Flags: review+

    
    

    
  
checked in for 1.3a
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED

    
    

    
  
*** Bug 181468 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 182542 has been marked as a duplicate of this bug. ***

    
    

    
  
this caused a regression, see bug 179412.

the regression has been fixed, but there's still a bug lurking.

see bug 183776 for that issue.

    
    

    
  
verifying
Status: RESOLVED → VERIFIED

    
    

    
  
*** Bug 197174 has been marked as a duplicate of this bug. ***
Component: HTML: Form Submission → DOM: Core & HTML

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: