Closed
Bug 510202
Opened 15 years ago
Closed 15 years ago
xlink:title should only work on links
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
People
(Reporter: jay, Assigned: longsonr)
References
()
Details
(Keywords: testcase)
Attachments
(3 files, 4 obsolete files)
5.65 KB,
patch
|
dao
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
844 bytes,
text/html; charset=utf-16
|
Details | |
5.53 KB,
patch
|
Details | Diff | Splinter Review |
mouse over the orange box. a tooltip is raised, NB this is not an anchor. Is this a bug? SVG WG discussion: http://lists.w3.org/Archives/Public/www-svg/2009Aug/0030.html one proposed use case is: onEvent script is used to import data using xmlhttprequest for this purpose title is (may?) not appropriate, ie similar issue to alt and title in html, title describes image, xlink:title describes linked resource.
Reporter | ||
Comment 1•15 years ago
|
||
neither safari nor opera mimic this behaviour.
OS: Mac OS X → All
Hardware: PowerPC → All
Assignee | ||
Updated•15 years ago
|
Component: SVG → General
Product: Core → Firefox
QA Contact: general → general
Assignee | ||
Comment 2•15 years ago
|
||
Assignee: nobody → longsonr
Attachment #426679 -
Flags: review?(dao)
![]() |
||
Comment 3•15 years ago
|
||
Note that the xlink:type thing is probably unnecessary and that this list would need to be expanded when we fix the mathml linking bug. Or something. In the C++ code, can we just QI to Link (at least once Shawn lands) instead of all the hardcoding?
Assignee | ||
Comment 4•15 years ago
|
||
I thought mathml links were just xmlns:type="simple". Is the mathml linking bug 427990? Erm. Once Shawn lands what?
Assignee | ||
Comment 5•15 years ago
|
||
http://www.w3.org/TR/REC-MathML/chapter7.html#sec7.1.5 was what I was looking at.
![]() |
||
Comment 6•15 years ago
|
||
> I thought mathml links were just xmlns:type="simple". They are, but not all xlink:type="simple" things are links. The Mathml linking bug is indeed bug 427990. Shawn is hopefully going to land bug 461199 and appurtenances thereunto today.
Assignee | ||
Comment 7•15 years ago
|
||
We can put a note in bug 427990 about changing this code when writing a patch for that. If bug 461199 lands I can rework the C++ code, if not I can raise a follow up and fix it once it does.
Attachment #426679 -
Attachment is obsolete: true
Attachment #426824 -
Flags: review?(dao)
Attachment #426679 -
Flags: review?(dao)
Comment 8•15 years ago
|
||
Comment on attachment 426824 [details] [diff] [review] remove link type code per bz comment >+ if ((tipElement instanceof HTMLAnchorElement && tipElement.href) || >+ (tipElement instanceof HTMLAreaElement && tipElement.href) || >+ tipElement instanceof HTMLLinkElement || I don't think the HTMLLinkElement line makes sense. <link> is always empty and only permitted in <head>, as far as I know... hence no way to invoke a tooltip. You'll need somebody else to review the embedding/ part.
Attachment #426824 -
Flags: review?(dao) → review+
Assignee | ||
Comment 9•15 years ago
|
||
Attachment #426824 -
Attachment is obsolete: true
Attachment #428068 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 10•15 years ago
|
||
I can either make the change so it looks like this once bug 461199 lands or alternatively ask Shawn to include this change when he lands bug 461199. Assuming it's right of course.
Attachment #428069 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•15 years ago
|
Attachment #428068 -
Attachment is patch: true
![]() |
||
Comment 11•15 years ago
|
||
> <link> is always empty Not necessarily. > hence no way to invoke a tooltip. Would you like a testcase that makes an <html:link> not only hoverable but also clickable as a link? The updated patches look ok to me except for the <link> issue and the fact that they will treat some things as links that aren't actually links (cases where the href attribute is set but can't be used to create an nsIURI). Not sure there's much you can do about the latter, but please fix the <link> thing.
Assignee | ||
Updated•15 years ago
|
Attachment #426824 -
Attachment is obsolete: false
Attachment #426824 -
Flags: superreview?(bzbarsky)
Assignee | ||
Comment 12•15 years ago
|
||
Comment on attachment 426824 [details] [diff] [review] remove link type code per bz comment link version resurrected
Assignee | ||
Updated•15 years ago
|
Attachment #428068 -
Flags: review?(bzbarsky)
![]() |
||
Comment 13•15 years ago
|
||
Comment on attachment 426824 [details] [diff] [review] remove link type code per bz comment You want to check @href for <link> as well, probably. sr=me with that.
Attachment #426824 -
Flags: superreview?(bzbarsky) → superreview+
![]() |
||
Comment 14•15 years ago
|
||
Comment on attachment 428069 [details] [diff] [review] post bug 461199 r=bzbarsky
Attachment #428069 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•15 years ago
|
Attachment #428068 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Summary: xlink is anchor only? → xlink:title should only work on links
Comment 15•15 years ago
|
||
(In reply to comment #11) > > <link> is always empty > > Not necessarily. > > > hence no way to invoke a tooltip. > > Would you like a testcase that makes an <html:link> not only hoverable but also > clickable as a link? A testcase would be interesting, since my reading of the html spec seemed to support my naive understanding.
Assignee | ||
Comment 16•15 years ago
|
||
checked in http://hg.mozilla.org/mozilla-central/rev/9d0a036ed2cb
![]() |
||
Comment 17•15 years ago
|
||
> A testcase would be interesting
Here you go. Note that the <link>s are visible in webkit and opera too, but they don't make them into actual clickable links. We do. If we stop doing that, we can stop treating them as links here too...
Also note that I could have used kids appended to the <link>s via the DOM or an XML testcase instead of CSS generated content.
Assignee | ||
Comment 18•15 years ago
|
||
Comment 16 fixed the bug, although I still need to land the follow up once bug 461199 lands.
Blocks: xlink-mathml
Status: NEW → RESOLVED
Closed: 15 years ago
Depends on: async-visited-check
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite+
![]() |
||
Updated•15 years ago
|
Attachment #428620 -
Attachment mime type: text/html → text/html; charset=utf-16
Assignee | ||
Comment 19•15 years ago
|
||
The follow up to use mozilla::dom::Link breaks non-libxul builds. nsCOMPtr<mozilla::dom::Link> linkContent(do_QueryInterface(currElement)); if (linkContent) { nsCOMPtr<nsIURI> uri(linkContent->GetURI()); results in the linker complaining that GetURI can't be found. I don't know what to do about that since adding SHARED_LIBRARY_LIBS = \ $(DEPTH)/content/base/src/$(LIB_PREFIX)gkconbase_s.$(LIB_SUFFIX) \ $(NULL) to the Makefile results in 1 link error becoming 238 link errors from the dependent libraries that gkconbase_s requires.
![]() |
||
Comment 20•15 years ago
|
||
Hmm. Shawn, can we expose the URI via some sort of virtual method (possibly in addition to GetURI) so this can work?
Comment 21•15 years ago
|
||
(In reply to comment #20) > Hmm. Shawn, can we expose the URI via some sort of virtual method (possibly in > addition to GetURI) so this can work? There should be some way to make linking work here, shouldn't we? If push comes to shove, we can always add some new interface I guess.
![]() |
||
Comment 22•15 years ago
|
||
The ways to make that linking work are to either inline the method, make the two things part of the same library, make it virtual (or add a GetURIExternal method that's virtual), or change visibility so it's globally exposed from the module... Usually in this situation we add a virtual *External method.
Comment 23•15 years ago
|
||
Why don't we just change the visibility then since that seems simplest: NS_EXTERNAL_VIS(already_AddRefed<nsIURI>) GetURI() const;
Comment 26•15 years ago
|
||
(In reply to comment #25) > Ideally NS_EXTERNAL_VIS would only be applied in dynamic builds. Yeah, maybe we could make a new define for that?
Assignee | ||
Comment 27•15 years ago
|
||
I can't get NS_EXTERNAL_VIS to link. virtual works a treat though.
Attachment #428069 -
Attachment is obsolete: true
Attachment #431837 -
Flags: superreview?(bzbarsky)
Attachment #431837 -
Flags: review?(sdwilsh)
Comment 28•15 years ago
|
||
Comment on attachment 431837 [details] [diff] [review] updated post bug 461199 patch you'll need to rev the Link CID too, right? r=sdwilsh
Attachment #431837 -
Flags: review?(sdwilsh) → review+
![]() |
||
Updated•15 years ago
|
Attachment #431837 -
Flags: superreview?(bzbarsky) → superreview+
Assignee | ||
Updated•15 years ago
|
Attachment #431971 -
Attachment description: port bug 46119 with iid rev → post bug 46119 with iid rev
You need to log in
before you can comment on or make changes to this bug.
Description
•