Closed Bug 551838 Opened 10 years ago Closed 10 years ago

Adding empty href attribute doesn't add link styling

Categories

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

x86
macOS
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla2.0b1

People

(Reporter: jruderman, Assigned: sdwilsh)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(3 files)

Attached file testcase (dynamic)
No description provided.
Attached file reference (static)
That's a bug in the GetHref() optimization in nsHTMLAnchorElement::SetAttr.

I still suspect we should just change that optimization to look at the cached URI instead.
Er, I can't seem to find where GetHref is defined.  Little help here?
171 NS_IMPL_URI_ATTR(nsHTMLAnchorElement, Href, href)
Banging my head a bit here.  How exactly does using the cached href help here?
This bug happens because GetHref returns "" when there is no href attr (or if it's somehow invalid).  In those cases mCachedHref would be null, so we wouldn't skip the notification.
(In reply to comment #6)
> This bug happens because GetHref returns "" when there is no href attr (or if
> it's somehow invalid).  In those cases mCachedHref would be null, so we
> wouldn't skip the notification.
OK, for whatever reason I wasn't getting that.  Got it now.  But do we really want to change how GetHref works, or do we just want to change how SetAttr checks this (by having it call GetURI instead)?
The latter, except we don't want to GetURI because that will set up mCachedURI even though we're about to blow it away.  Hence my comments about just using mCachedURI directly if we can...  Or just removing this GetHref check entirely.  Or something.
Attached patch v1.0Splinter Review
I think this is will be acceptable to you (and I'm happy that I didn't expose mCachedURI :) )
Attachment #432841 - Flags: review?(bzbarsky)
Comment on attachment 432841 [details] [diff] [review]
v1.0

r=bzbarsky.  Sorry for the ludicrous (going on plaid) lag.
Attachment #432841 - Flags: review?(bzbarsky) → review+
Whiteboard: [can land]
http://hg.mozilla.org/mozilla-central/rev/5fe1e70ee300
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [can land]
Target Milestone: --- → mozilla1.9.3a5
Target Milestone: mozilla1.9.3a5 → mozilla1.9.3a6
Interesting failure seen in this test now on an OS X debug box.  Basically, the snapshot of the test image gets taken after we've determined that "test anchor 3" and "test link 3" are in fact visited (this page).  I'm guessing this is because debug builds are slow so there's more time for that db lookup to happen because we are running script.
Depends on: 572539
Depends on: 572576
You need to log in before you can comment on or make changes to this bug.