Closed
Bug 550661
Opened 14 years ago
Closed 14 years ago
Adding href attribute doesn't add link styling
Categories
(Core :: DOM: Core & HTML, defect, P1)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a3
| Tracking | Status | |
|---|---|---|
| blocking2.0 | --- | final+ |
People
(Reporter: jruderman, Assigned: sdwilsh)
References
Details
(Keywords: regression, testcase)
Attachments
(3 files)
No description provided.
|
Reporter | |
Comment 1•14 years ago
|
||
|
|
Reporter | |
Comment 2•14 years ago
|
||
(The opposite bug was fixed in bug 549797.)
|
||
Updated•14 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
|
Assignee | |
Comment 3•14 years ago
|
||
The fix is not going to be the same. I'm a bit confused as to why this isn't working... (I also thought we had a test for this)
|
||
Comment 4•14 years ago
|
||
> I'm a bit confused as to why this isn't working
Because when we do the ResetLinkState() at the end of nsHTMLAnchorElement::SetAttr, we end up in Link::ResetLinkState with mLinkState == eLinkState_NotLink, so we set mLinkState to defaultState but don't notify on that state change. Why not?|
|
Assignee | |
Comment 5•14 years ago
|
||
(In reply to comment #4) > > I'm a bit confused as to why this isn't working > > Because when we do the ResetLinkState() at the end of > nsHTMLAnchorElement::SetAttr, we end up in Link::ResetLinkState with mLinkState > == eLinkState_NotLink, so we set mLinkState to defaultState but don't notify on > that state change. Why not? There was a reason for that, but I don't know what it was or if it is relevant anymore. I'll try removing that path and add some tests and see what happens.
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.9.3a3
|
|
Assignee | |
Comment 6•14 years ago
|
||
(In reply to comment #5) > There was a reason for that, but I don't know what it was or if it is relevant > anymore. I'll try removing that path and add some tests and see what happens. That was because we would assert when trying to tell the document to forget us, but that was fixable. Now I'm failing my test, and I don't understand why...
|
|
Assignee | |
Comment 7•14 years ago
|
||
(In reply to comment #6) > That was because we would assert when trying to tell the document to forget us, > but that was fixable. Now I'm failing my test, and I don't understand why... Buggy test :) I have a fix in hand that I'm going to push to the try server and upload in just a moment.
|
|
Assignee | |
Comment 8•14 years ago
|
||
Will request review once I get positive try server results. Not 100% sure that we don't break other tests.
|
|
Assignee | |
Updated•14 years ago
|
Attachment #431236 -
Attachment is patch: true
Attachment #431236 -
Attachment mime type: application/octet-stream → text/plain
|
|
Assignee | |
Comment 9•14 years ago
|
||
+ s/removal/change/ in the test filenames
|
|
Assignee | |
Comment 10•14 years ago
|
||
Comment on attachment 431236 [details] [diff] [review] v1.0 Try server says we are OK.
Attachment #431236 -
Flags: review?(bzbarsky)
|
|
Assignee | |
Updated•14 years ago
|
Flags: in-testsuite?
Flags: in-litmus-
|
|
||
Comment 11•14 years ago
|
||
Comment on attachment 431236 [details] [diff] [review] v1.0 r=bzbarsky with those renames
Attachment #431236 -
Flags: review?(bzbarsky) → review+
|
|
Assignee | |
Comment 12•14 years ago
|
||
Those renames are in my patch queue: http://hg.mozilla.org/users/sdwilsh_shawnwilsher.com/storage-async/file/2f2e9460534b/fix-link-reg Will push tomorrow if the tree is OK.
|
|
Assignee | |
Comment 13•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/c4b4efecdaee
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
|
||
Updated•14 years ago
|
blocking2.0: ? → final+
You need to log in
before you can comment on or make changes to this bug.
Description
•